[SOLVED] Is this code technically correct

[purencool]

hi phpfreaks,

 

My class is doing procedural  php. But I have been doing that for years. I really want to do oop so I sat down and wrote the following below. What I want to know is it technical correct? I am sorry about the length but I want everyone to have a complete picture.  Any help would be great

 

Firstly the form

                   <h3>How long will it take to have enough to retire?</h3>
                          What is the amount you want to retire with? (eg:1000) 
                          $<input type="text " name="endAmount" value="1000000" size="10"/>
                           What is you intial contribution? (eg:100)
                           $<input type="text " name="intialAmount" value="10000" size="10"/>
                           At what interest rate (eg:15)
                             <input type="text " name="interest" value="15" size=4" />%
                   <input type="submit" />

 

Second is the creation of the object

if($_GET['endAmount'] != '' || $_GET['intialAmount'] != '' || $_GET['interest'] != '' ){   
    $end = $_GET['endAmount'];
    $inital =$_GET['intialAmount'];
    $interest = $_GET['interest'];
   echo "$end , $inital, $interest";
    echo"<h2>Investment Return</h2>";  
    
    require_once("classes/ChFourInvest.php");
    $returnInvestObj = new  ChFourInvest($interest,$inital,$end);
    
    echo" 
        <p>The Amount of money you want is: $end</p>
        
        <p>You want to do this by the following formula,</p>
        <dl>
        <dt>The interest will be set at: $interest</dt>
        <dt>The initial amount you want to provide is: $initial</dt>
        </dl>
        <p>This will take you <b>" . $returnInvestObj -> yearTaken() . "</b> years </p>
        ";
}

 

Thirdly is the class itself

class ChFourInvest
{
    public $interest; 
    public $initialInvestment;
    public $endAmount;
    
    public function  __construct($interest,$initalInvestment,$endAmount)
    {
        
        $this->interest = $interest;
        $this->initialInvestment = $initalInvestment;  
        $this->endAmount = $endAmount;
    } 
    public function yearTaken (){
            $yearTaken = 0;
            $initialinvestX = $this->initialInvestment;
            $runningTotal = 0;

            do {
                    $yearTaken++;   
                    $runningTotal = $this->interest / 100 * $initialinvestX;           
                    $initialinvestX = $runningTotal + $initialinvestX;
                }while( $initialinvestX <= $this->endAmount );
                
         return $yearTaken;    
        }
  
  }

 

[chronister]

I don't see anything wrong with it. It is structurally sound, and it appears you are using methods and properties the correct way.

 

I think the only thing I would change is to set the __construct to a private method. Since you are taking the time to designate it as public, private or protected set it to private as it does not need to be accessible directly.

 

I could be wrong.

 

But I have tested this and the construct will run properly when set as private. It will not run if set to protected though.

 

Nate

[purencool]

Thanks Nate

 

I was little nervous that I had it all wrong I will try what you suggested

[Bendude14]

You do not want your constructor to be private or methods that you would like to use from outside of the class.

 

Google, access modifiers to learn a bit more about this.

 

You should set your data to private so it can only be accessed through getter and setter methods you create. Encapsulation is a key part of OO programming.

 

PHP has some "magic" methods you may want to look into

 

__get() and __set()

 

http://au.php.net/manual/en/language.oop5.overloading.php

[purencool]

Thanks I will look it up

 

You do not want your constructor to be private or methods that you would like to use from outside of the class.

 

Does this mean if I don't want anyone to access a method from the outside the class I do the following. So when I create the object and call a private method it will be ignore by the php engine or through and error?

private function grossWages (){
}

[purencool]

Ooh I tried this and error so now I have to use the  __set() initialize the constructor instead that is the theory is this correct?

 

    private function  __construct($interest,$initalInvestment,$endAmount)
    {
        
        $this->interest = $interest;
        $this->initialInvestment = $initalInvestment;  
        $this->endAmount = $endAmount;
    } 

[purencool]

I have made some changes to the code. Is this technically correct?

This the  object

if($_GET['endAmount'] != '' || $_GET['intialAmount'] != '' || $_GET['interest'] != '' ){   
    $end = $_GET['endAmount'];
    $inital =$_GET['intialAmount'];
    $interest = $_GET['interest'];
    echo"<h2>Investment Return</h2>";  
    
        require_once("classes/ChFourInvestA.php");
        $returnInvestObj = new  ChFourInvest();
        $returnInvestObj -> setInterest($interest);    
        $returnInvestObj -> setInitialInvestment($inital); 
        $returnInvestObj -> setEndamount($end);
    
    echo" 
        <p>The Amount of money you want is: $end</p>
        
        <p>You want to do this by the following formula,</p>
        <dl>
        <dt>The interest will be set at: $interest</dt>
        <dt>The initial amount you want to provide is: $initial</dt>
        </dl>
        <p>This will take you <b>" . $returnInvestObj -> yearTaken() . "</b> years </p>
        ";
}

This is the class

 class ChFourInvest
{
    private $interest; 
    private $initialInvestment;
    private $endAmount;
    

    
    public function yearTaken (){
            $yearTaken = 0;
            $initialinvestX = $this->initialInvestment;
            $runningTotal = 0;

            do {
                    $yearTaken++;   
                    $runningTotal = $this->interest / 100 * $initialinvestX;           
                    $initialinvestX = $runningTotal + $initialinvestX;
                }while( $initialinvestX <= $this->endAmount );
                
         return $yearTaken;    
        }
    
    public function setInterest($interest){     
        $this->interest = $interest;
    }
  
     public function setInitialInvestment($initalInvestment){     
        $this->initialInvestment = $initalInvestment;  

    }
     public function setEndamount($endAmount){     
        $this->endAmount = $endAmount;
    }
  }

[purencool]

Is this a technically correct object orientated php class?

class ChFourInvest
{
    private $interest; 
    private $initialInvestment;
    private $endAmount;
    

    
    private function yearTaken (){
            $yearTaken = 0;
            $initialinvestX = $this->initialInvestment;
            $runningTotal = 0;

            do {
                    $yearTaken++;   
                    $runningTotal = $this->interest / 100 * $initialinvestX;           
                    $initialinvestX = $runningTotal + $initialinvestX;
                }while( $initialinvestX <= $this->endAmount );
                
         return $yearTaken;    
        }
    
    public function setInterest($interest){     
        $this->interest = $interest;
    }
  
    public function setInitialInvestment($initalInvestment){     
        $this->initialInvestment = $initalInvestment;  

    }
    public function setEndamount($endAmount){     
        $this->endAmount = $endAmount;
    }
    public function getAmountOfYears (){
         return $this->yearTaken ();
    }
  }
  

 

 

[Bendude14]

That is how you would use getters and setters but if everytime you create a CHFourInvest Object you will be setting $interest, $initial and $end then you should still be passing these into the constructor like you were before.

 

A constructors purpose is to initialise the object, "like a preparation stage".

 

getters and setters give you access to private data, in case you need to update the data or retrieve the data from the object later on.

 

You provide getters and setters because other people may not no how to edit your data correctly and should therefore use the methods provided.

 

Yes making a method private would only allow access to it from within the same class

 

Did you look at the php __set() and __get() will save you some time in the long run

[Bendude14]

your yearTaken method should be public....

[chaiwei]

using __set and __get will bring any side effects?

I saw some one post using this magic method will bring some security issues?

 

Or may be they using __set and __get method was using incorrectly?

 

function __set($var, $val){
   switch($var){
     case 'name':
          $this->$name=$val;
          break;
     default:
         echo "You can't set this $var in this class";
   }
}

 

is this the correct?

 

[Bendude14]

personally i do like to write my get() and set() methods as I find myself writing a lot of java.

 

Here is a small intro to __get() and __set()

 

http://www.sunilb.com/php/php-tutorials/php5-oops-tutorial-__get-and-__set

 

As for the security issues, I am not aware of any.

 

Ben

[purencool]

Sorry I have been a work thanks for all the information somethings I am still trying to get around

[purencool]

Ok

I think I have worked this out. If a variable is being sent to the class it will automatically goto the __set method and then that method will initializes the private variables. Is this correct?

 

This is what I have done so far is this technically correct?

When using the _set method everyone uses arrays when placing multiple

variables into the _set method do you have to do this?

class ChFourInvest
{

     private $data = array();


     public function __set($name, $value) {
          echo "Setting '$name' to '$value'<br />";
             $this->data[$name] = $value;
         }
     public function __get($name) {
             if (array_key_exists($name, $this->data)) {
                     return $this->data[$name];
                 }
         }

}

        $returnInvestObj = new  ChFourInvest();
        $returnInvestObj->initialInvestment = $inital;
        $returnInvestObj->endAmount=$end;
        $returnInvestObj->interest=$interest;
        echo  $returnInvestObj->initialInvestment . "<br />";
        echo  $returnInvestObj->endAmount . "<br />";
        echo  $returnInvestObj->interest . "<br />";

 

[Bendude14]

using an array would be the best way to do it, but you don't have to do it. One problem may be if you did not use an array would be that if someone else was using your class they could try to set there own data and overwrite your data inside the class.

 

Yes the set method is called if the variable does not exist but also if you are in the wrong scope, eg if you were trying to access private data inside your class the get method would be called.

 

Your code is technically correct, good luck with the rest of the project.

 

Ben

 

PS: I would set them 3 vars inside the constructor if you intend to use them for every CHFourInvest Object. Remember to store them inside the data array.

[purencool]

Thanks it took some real effort to get this right.

 

 

I have one question how do I use the _set function and

place it in the constructor? Because Bendude14 is right I will be using them in

other methods

 

This is the completed class

 class ChFourInvest
{
    // The investment variable in the array need to be
    // initialInvestment, interest and endAmount
     private $investmentVar = array();
     


     public function __set($name, $value) {
             $this->investmentVar[$name] = $value;
         }
         
     public function yearTaken (){
             $yearTaken = 0;
                 $initialinvestX = $this->investmentVar[initialInvestment];
                 
             $runningTotal = 0;
         
         
             do {
                 $yearTaken++;   
                 $runningTotal =  $this->investmentVar[interest]/ 100 * $initialinvestX;           
                 $initialinvestX = $runningTotal + $initialinvestX;
                     }while( $initialinvestX <= $this->investmentVar[endAmount] );
         
             return $yearTaken;    
         }

}

[Bendude14]

This should do it...

 

<?php

private $data = array();

public function __construct($initialInvestment, $interest, $endAmount)
{
    $data[$initialInvestment] = $initialInvestment;
    $data[$interest]             = $interest;
    $data[$endAmount]        = $endAmount;

}
?>

[purencool]

Thanks

Why do you place the variables in the constructs brackets does the construct make them global to the class if placed there?

public function __construct($initialInvestment, $interest, $endAmount)

 

 

[Bendude14]

the constructor is just another method, well sort of. It is executed when you create an instance of the object. if your constructor takes two parameters then when you create the object you must pass in two parameters like so

 

public __construct($initial, $endAmount)
{

}


$obj = new CHFourInvest( $initial, $endAmount );

 

They are just values passed into the object, you then have to set them as you need, it does not make them global.

[purencool]

Thanks very much you do make it easy to understand oo