Page 1 of 2

Coding Style: passing values to functions

Posted: Thu Jun 12, 2003 5:50 am
by patrikG
Personally, I prefer doing the whole passing values to functions-business with associative arrays - but I am wondering about the pros and cons - as I haven't had to do any mission-critical stuff and write very performance-oriented code.

How do you do it (pass values to functions that is :P) ?

1. Do you use the rather linear

myFunction($value1, $value2, $value3, $value4, etc.)

2. or do you do it in OOP exclusively and set the properties before the function call?

$myClass->property1="bla1";
$myClass->property2=bla2;
$myClass->property2="bla3";
$myClass->myFunction();

3. or do you do it with arrays

$myArray=array("myValue1"=>"value1","myValue2"=>"value2");



To me the greatest benefits in using associative arrays is that
a) very easy to pass values from a form-submit to a function
b) it's a rather clean method imho as it keeps the code clean and makes debugging easy (better than 1.) and it doesn't bloat the code unnecessarily (better than 2.)
c) many more options in terms of data-accessability than OOP (see array-functions) - e.g. if necessary "array to oop" conversion is done with two lines of code, but "oop to array" - yikes.

Whatcha think?

Posted: Thu Jun 12, 2003 6:08 am
by cactus
I generally use (and tutor people to use) both arrays and objects, I hate to see a function/method with 20 zillion parameters (as per your "linear" example).

Code: Select all

$obj->var1->var2['DATA']
Obviously this is'nt a solution to all scenarios esp. when you need to set a default value:

Code: Select all

myFunction(var1, var2=FALSE)
But packaging your vars in to a well indexed array saves you no end of hassle and would be my preffered option.

:)

Re: Coding Style: passing values to functions

Posted: Thu Jun 12, 2003 8:20 am
by nielsene
I prefer a none-of-the-above style (or maybe a combination style): I'll tend to use a "linear" approach, but with class or array components when appropriate. Comments about your other two paradigms are included below.
patrikG wrote: [2. or do you do it in OOP exclusively and set the properties before the function call?

$myClass->property1="bla1";
$myClass->property2=bla2;
$myClass->property2="bla3";
$myClass->myFunction();
Yuck, to me.I hate public properties.... I've I'm doing something OOP style, I'll tend to use Get/Set's or I'll use the "property" feature of some languange (not PHP) that automatically converts attribute assignment to get/sets.
3. or do you do it with arrays

$myArray=array("myValue1"=>"value1","myValue2"=>"value2");
I use this very often when I'm prototyping. However, in later phases this is normally a sign that I need a new class to encapsulate this data structure. If the three values( in your example) are completely unrelated and not used my other functions as a lump, then I would stick with what you labelled linear, because as cactus mentioned it allows defaults to be set and presents a more useful function prototype.

Posted: Mon Jun 16, 2003 2:17 pm
by llimllib
I mostly agree with nielsene, I prefer the linear style unless the variables are related. If you're passing too many unrelated variables to a function, this should be a sign to you that you need to refactor your design. Use an associative array if the variables make sense, design-wise, to be in an array, but don't abuse it as a parameter-passing mechanism. This will make your functions unclear and difficult to understand for someone else, or yourself in a few months.

Posted: Thu Jul 10, 2003 10:06 pm
by macewan
cactus wrote:I generally use (and tutor people to use) both arrays and objects, I hate to see a function/method with 20 zillion parameters (as per your "linear" example).

Code: Select all

$obj->var1->var2['DATA']
Obviously this is'nt a solution to all scenarios esp. when you need to set a default value:

Code: Select all

myFunction(var1, var2=FALSE)
But packaging your vars in to a well indexed array saves you no end of hassle and would be my preffered option.

:)
man i've got a long journal ahead

:wink:

Posted: Fri Jul 11, 2003 8:40 am
by ik
Yes, associative array are very useful and flexible. Actually the can be used as array fields instead of list of variables and complex constructors.

I use smth like this:

Code: Select all

<?php


class myClass {
$var prop="";
    function myClass(&$prop="") {
        $this->prop=$prop;
    }

    function Foo() {
        if ($this->prop["address"]=="myaddress") {
          dosmth();
        } 
   }

}

....

$prop=array(
  "address"=>"ssss",
  "name"=>"sdsfdsf"
);

$x=new myClass(&$prop);


?>

Posted: Fri Jul 11, 2003 9:10 am
by jason
Several problems with this code:

Code: Select all

&lt;?php
class myClass {
$var prop="";
    function myClass(&amp;$prop="") {
        $this-&gt;prop=$prop;
    }

    function Foo() {
        if ($this-&gt;prop&#1111;"address"]=="myaddress") {
          dosmth();
        }
   }

}

....

$prop=array(
  "address"=&gt;"ssss",
  "name"=&gt;"sdsfdsf"
);

$x=new myClass(&amp;$prop);

?&gt;
First is the excessive use of '&'. You don't need it here '
$x=new myClass(&$prop);', as you have it in the function definition. Secondly, you don't really need it at all. Your passing it by reference, which takes time in PHP. Then, finally, when you are in the constructor, you do ' $this->prop=$prop;', which does the copy anyways. So simply remove all the '&' from your script, and it will work exactly the same way, except faster.

Remember, PHP does copy-on-write, which means if I do:

Code: Select all

function name ( $var )
{
    echo $var;
}

$var = "really long string";

name($var);
PHP doesn't make a copy of $var in the function. No need. It only makes a copy if you actually write to the variable. Otherwise, it basically uses a reference. When you actually write to the variable, it will make a copy. This is for memory issues, etc. If you are passing object, pass them by reference implicitly. Otherwise, you are probably wasting time.

Next, passing an array to a function to replace a long parameter list is a poor mans technique for not using objects. We have objects in PHP. Use them.

Rather than use your array, which means code maintenance is much more difficult, etc, you an object.

Code: Select all

<?php

class myClass 
{
	var $prop;
    
	function myClass( &$prop ) 
    {
        $this->prop =& $prop;
    }

    function Foo() 
    {
        if ($this->prop->getAddress() == "myaddress") {
        	dosmth();
        }
   }

}

class Person
{
    var $address;
    var $name;
    
    function Person () {}
    
    function setAddress ( $address ) 
    {
    	$this->address = $address;
    }
    
    function setName ( $name )
    {
    	$this->name = $name;
    }
    
    function getAddress ()
    {
    	return $this->address;
    }
    
    function getName ()
    {
    	return $this->name;
    }
}

$Person = new Person();
$Person->setAddress("Address");
$Person->setName("Jason Lotito");
$myClass = new myClass($Person);

?>
This is more or less what you want. Rather than pass an array that can change over time, you pass an object for which you control the interface.

Using an array instead of a long parameter list has several problems. First and foremost is that it takes even more work to pass the array than it does to use the long paramater list.

Secondly, it's usually an indication that your parameter list is just too long to begin with. A long paramter list is usually a sign that you don't know what you are doing (that you don't understand the problem). A good function should do one thing, and do it well. This rarely requires a lot of parameters. When it does, that's usually when you should go about making an object.

Thirdly, a long parameter list is usually a sign to a long function. Long functions are bad, because they do more than one thing. A function should be short and simple. A function that is too long can be broken up into smaller pieces.

my 2 cents

Posted: Fri Jul 11, 2003 9:44 am
by pootergeist
Wouldn't it advance the structure one more level to still pass a single associative index to a single function rather than the long list of setters?

Code: Select all

function setParam($var,$val)
 { 
 eval("\$this->" .$var. " = " .$val. ";);
 }

function getParam($var)
 {
 eval("return \$this->" .$var. ";);
 }
at least then if you distributed your class the users would only need to learn the getter and setter calls and a list of variable arguments that could be passed.
As it is your class users would need to understand what setName, getName, setAddr and getAddr referenced (yeah easy with self documenting code, not so easy with 20+ setters and getters even if the words mean something)

subnote: back on topic: I have used associative arrays in a constructor to bunch relative values together - eg
class b
{
function b($ref="",$c = array('colourA'=>'AAAAAA','colourB'=>'BBBBBB'))
{

which allows an array to be built and passed eg new b('my_ref',$my_array) or even build within the call - new b('myref',array('CCCCCC','EEEEEE')); and allows no variable for the array and hence default - new b('myRef');

Posted: Fri Jul 11, 2003 9:52 am
by pootergeist
yay - smileys in the php code :)

the main point I try to edge toward is allowing defaults and structuring them so the most likely occur first and least likely at the end

function history_notes($date,$country='uk',$person='king')

so
history_notes(1066) would be fine and tell about the entire world in 1066
history_notes(1066,'france') would tell about france in 1066
history_notes(1066,'france','bruce kibbleright') would probably say nowt.

function history_notes($person='king',$country='uk',$date)
obviously wouldn't work quite as well. (dependent upon what you're seeking)

basically, if you are going to default any arguments parameters of a function call, make sure the needed ones are first.

Posted: Fri Jul 11, 2003 10:12 am
by jason

Code: Select all

function history_notes($person='king',$country='uk',$date)
Well, that would be an error in any case. It doesn't parse.

As far as having the global set($var_name, $value), you could do that, however, the person still have to do the leg work of figuring out what paramters you want.

The problem here is that rather than have a nice list of methods you can use, you have to constantly look up what they are calling the variable. Do I want to do set('email', $email_address); or do set('email_address', $email_address). With proper setParam() and getParam(), I can avoid this, especially when the person is using an IDE. Rather than have to remember all my parameter names, for example, Zend knows them all. So, as I am typing, let's say $User->set, Zend will popup with all the set functions from the User class. That way, I don't have to worry about whether I named it setEmailAddress, or setEmail. That's just an additional feature.

Another problem with the set() method is like the naming issue. set('emal', 'jason@devnetwork'); won't display a parse error, though it is wrong. however, setEmalAddress() is wrong, and will display a parse error, resulting in quicker debugging.

Posted: Fri Jul 11, 2003 10:14 am
by ik
jason wrote: Several problems with this code:

Code: Select all

<?php
class myClass {
$var prop="";
    function myClass(&$prop="") {
        $this->prop=$prop;
    }

    function Foo() {
        if ($this->prop["address"]=="myaddress") {
          dosmth();
        }
   }

}

....

$prop=array(
  "address"=>"ssss",
  "name"=>"sdsfdsf"
);

$x=new myClass(&$prop);

?>

First is the excessive use of '&'. You don't need it here '
$x=new myClass(&$prop);', as you have it in the function definition. Secondly, you don't really need it at all. Your passing it by reference, which takes time in PHP. Then, finally, when you are in the constructor, you do ' $this->prop=$prop;', which does the copy anyways. So simply remove all the '&' from your script, and it will work exactly the same way, except faster.
Well, I was certainly wrong with the reference. In fact I am using simple copying - I have added reference just when was typing 8O - to do it looking better...

But I'll try to discuss futher about other things.
First - I haven't meant "long parameter list" for function - but for class fields.

Second. An equivalence of class fields and associative array is already implemented in the number of poor typed interpeted languages such as JavaScript and Matlab. Moreoever it's partially implemented in PHP - for class methods. The reason is obvious - in interpereted languages any field called by name in fact. It is not so for strongly typed languages as C++ an so on.
PHP doesn't make a copy of $var in the function. No need. It only makes a copy if you actually write to the variable. Otherwise, it basically uses a reference. When you actually write to the variable, it will make a copy. This is for memory issues, etc. If you are passing object, pass them by reference implicitly. Otherwise, you are probably wasting time.
Yes that's correct. But I had no idea to pass this array to class methods - it is accessible for them as $this->prop field.

Code: Select all

<?php
class Person
{
    var $address;
    var $name;
    
    function Person () {}
    
    function setAddress ( $address ) 
    {
    	$this->address = $address;
    }
    
    function setName ( $name )
    {
    	$this->name = $name;
    }
    
    function getAddress ()
    {
    	return $this->address;
    }
    
    function getName ()
    {
    	return $this->name;
    }
}

$Person = new Person();
$Person->setAddress("Address");
$Person->setName("Jason Lotito");
$myClass = new myClass($Person);

?>
Yes. But my point was that is exactly the same what I have written. The only difference is possibility to add "pseudofields" on fly - just as in JavaScript.

Posted: Fri Jul 11, 2003 10:27 am
by jason
ik: Ahh, okay, I think I understand what you were talking about.

Rather than implicitly do:

Code: Select all

$obj->setEmailAddress($_POST['email_address');
for let's say, 20 parameters, you can do:

Code: Select all

$obj->setFromPost($_POST);
and be done with it. Am I correct in this? If so, then yes, I most assuredly do agree.

Posted: Fri Jul 11, 2003 10:51 am
by ik
Yes, that's right.
But the main point is the possibility to deal with unknown beforehand and arbitrary number of parameters.

Real example. I need to draw a table with data from some array $data[]. Instead of this array I shuold use some style parameters: class, cellspasing and so on. All of them goes to <table...> tag with the same notation: "prop"="value". The number of parameters unknown and can be set by user. Wihout arrays I should set a lot of fields:

Code: Select all

class TGrid&#123; 
var $data;
var $class;
var $cellspasing;
....
&#125;
or use preformated string with all of them to pass to constructor.
WIth arrays I do:

Code: Select all

<?php


class TGrid {
var $data;
var $prop;
....
}

....
$prop=array(
  "class"=>"mytab",
  "cellspacing"=>"1",
   ....{whatever}
);

$Grid=new TGrid($data,$prop);


?>

Posted: Fri Jul 11, 2003 11:15 am
by nielsene
However, I would argue it may be better to define a setProperty function that takes a number of arguments with some defaulting. Otherwise you might end up with people passing in mispelled labels ('celspacing'), etc and wondering why their table looks wrong. You also make is very tough to make your generated pages validate according to a DTD as you can't know what possible unallowed/deprecated attributes the user is going to pass in.

Or you could use a PropertyObject in the first place and pass that to the your Grid object, etc. Which is propably the better solution in this case. You could still pass in your associative array to it, but it knows what's legal and not and siliently strips out bad inputs, etc.

Going off-topic....

Two common OOP fallacies:
1. Every attribute must have its own get/set
2. Every non get/set method takes to arguments

Addressing 1: Yes normally every attribute will have its own get/set functions, but sometimes attributes should be updated as a pair, in which case you would have one set with two parameters, with the setter ensuring the proper correspondence between the variables. Also, the attributes may change, but ideally the interface won't. This is another argument for not having an exact 1-1 correspondence between attributes and methods. If you have the 1-1 correspondence you're likely to want to break the interface when you change the internal respresentation

Address 2: I see a lot of code like

Code: Select all

$x = new Foo();
$x->setBar("stuff");
$x->setBaz("moreStuff");
$x->doQuz();
, with no other use of x in the code. To me this looks like a novice attempt at OOP. Why aren't stuff/moreStuff arguements to either the constructor or doQuz? Are bar/baz variables that affect the inner-workings of the class or only one function? If the former, they should probably be allowed to be set in the consttructor (with defaults), if the latter they should be set in the doQuz call...

Personally I see use of numerous get/set's a sign of poor class design and poor interpretation of OOP.

Posted: Fri Jul 11, 2003 11:44 am
by ik
nielsene wrote:However, I would argue it may be better to define a setProperty function that takes a number of arguments with some defaulting. Otherwise you might end up with people passing in mispelled labels ('celspacing'), etc and wondering why their table looks wrong. You also make is very tough to make your generated pages validate according to a DTD as you can't know what possible unallowed/deprecated attributes the user is going to pass in.

Or you could use a PropertyObject in the first place and pass that to the your Grid object, etc. Which is propably the better solution in this case. You could still pass in your associative array to it, but it knows what's legal and not and siliently strips out bad inputs, etc.
[/qoute]

Yes, I agree. Unforunately PHP classes don't have private fields/methods, so user is allowed to directly rewrite any field anyway. And in my case user is the same person as class writer :D
Going off-topic....

Two common OOP fallacies:
1. Every attribute must have its own get/set
2. Every non get/set method takes to arguments
Well, I am not pretty sure about both :D
Especially about 2. Foo->Run() is a quite common construct in any language.