...

View Full Version : Never passing args to a class = bad?



cfructose
04-03-2010, 09:45 AM
I've just started using .ini.php files, as discussed in this recent thread:

http://www.codingforums.com/showthread.php?p=940167#post940167

My class is for handling a comments/guestbook form.
It starts:


class Form extends Database{
private $ini, $formType, $formErrors, $toPost;

public function __construct($formType, $db) {
parent::__construct($db);

$this->formType = $formType;
$this->ini = parse_ini_file(CONFIG.$formType.".ini.php", TRUE);
}
//...
}

What's worrying me is that now that I'm using parse_ini_file, I've got nothing left to pass to the methods of this class. The class already contains all the properties it needs to do everything by itself!

I mean, $this->ini is an array containing all the variables this class needs that are configurable - everything from the sepcifications of the form's fields, to the structure of its database, to the rules for paginating comments when they're displayed.

Consequently, where I used to have a config array, parts of which I would pass as args like this:


$numberPerPage = $form->getPagination($config['comments']['numberPerPage']);

...Now I just have:


$numberPerPage = $form->getPagination();

And where I used to only access methods if certain conditions were met, like:



$country = ($config['comments']['useGeoIP'] == TRUE) ? $form->getCountryFromIP($ip) : '';


...now, given that these conditions can only be checked inside the class, I just do:


$country = $form->getCountryFromIP();

Is there anything wrong with never passing args to a class? It feels wrong, but I'm simply too inexperienced with OOP to know.

It gets worse: As $_POST is a superglobal, I don't even have to pass that to the class, so after letting my __construct know where to find its config file, there's really nothing else I need to pass to this class at all.

That can't be right/good, can it?
But why? :confused:

Dormilich
04-03-2010, 03:41 PM
why should it be wrong not passing things you don’t need?

MattF
04-03-2010, 05:56 PM
Half the fun of classes is the fact of not always having to pass vars in. :D

cfructose
04-03-2010, 08:02 PM
OK, that's reassuring.
:)

>Why should it be wrong?

Well, apparently it isn't! Look, before I drop this idea, let me use an example to take it one step further... This might help elucidate why I feared there might be something 'wrong' with this, but you both may well still repeat that this is still not only fine, but one of the joys of OOP.

Given that the class already knows everything it needs to know, I don't even have to return anything form any of its methods. So, rather than:


$country = $form->getCountryFromIP();
$numberPerPage = $form->getPagination();

...I can just do:



$form->getCountryFromIP();
$form->getPagination();

...where what these methods calculate can simply be stored in the object as $this->country and $this->numberPerPage.

But having reached this stage, it seems pointless to have a long series of calls to methods in a class like this:


$form->method();
$form->anotherMethod();
$form->yetAnotherMethod();
//etc

...when I could just call a single method that's responsible for calling all the others:



class Form {
//...
function GodMethodThatDoesEverything(){
$this->method();
$this->anotherMethod();
$this->yetAnotherMethod();
}
//...
}

$form->GodMethodThatDoesEverything();

That way, I have one fewer scripts to worry about for any future project that require this class: outside the class, all I have to do is create a .ini.php file, include the class, instantiate an object, and call the 'God' method.

So you see, that starts to feel more and more like procedural thinking: a rigid, foreordained order that benefits nothing from having being OO in the first place. Does that help you see why I feared that parameter-less methods might be 'bad'?

If I'm talking nonsense, and there's nothing either unusual or 'bad practice' about what I've just described, then I'll take you at your word for now (until the day comes that I get it intuitively). I'm just a little paranoid that my attempts at OO code are nothing but a procedural mentality dressed up with some new nomenclature.

Dormilich
04-03-2010, 08:09 PM
that GodMethodThatDoesEverything() is usually called __construct(). in the end it also depends on what tasks the object has to fulfil, so I can’t say much for your case. there might be the case one day, where you explicitly need to get the Country from IP, but you have a method for that …

public methods should always return something (be it even true/false, unless you have reason not to)

cfructose
04-03-2010, 08:36 PM
that GodMethodThatDoesEverything() is usually called __construct()

Ah, that helps a lot. Thanks.



it also depends on what tasks the object has to fulfil [...] there might be the case one day, where you explicitly need to get the Country from IP

Yeah, should I ever happen to want to make use of any of the methods individualy, or in a different order, I can always access them from outside the class, sure. As you point out, getting the IP, for instance, is still possible.

But that's why I suggested this 'God method'. If I put calls to all the methods in __construct, then I no longer have the choice of over-riding the default behaviour.


public methods should always return something (be it even true/false, unless you have reason not to)

Ah, I think that clinches it for me. It's starting to make sense...
If I store a boolean in a property of $this as opposed to returning a boolean, then there's no way to access that value from outside the class. (Or is there?)

Is there any syntax that would make it possible to achieve:


$validationPassed = $form->validate();

...by requesting a property, rather than a method, something like:


$validationPassed = $form->validationPassed;

?

If not, then these methods are no longer useful for anything but the 'default behaviour' unless I return something. I get it now.

Dormilich
04-03-2010, 08:41 PM
I would make a value into a public property when, er, it is required by the Interface or you need to access it often but only need to calculate it once.

not to forget lazyness.

ah, and your last question, that would be getters (__get()), although they should be used carefully

cfructose
04-03-2010, 08:54 PM
I would make a value into a public property when, er, it is required by the Interface

I take it from your "er" that this statement is to be taken with a pinch of salt. I'm not sure I understand. :confused: I don't suppose you could elaborate on 'required by the Interface'? When is something not?


or you need to access it often but only need to calculate it once.

This, I do understand. That makes complete sense. If I keep this idea of 'access often, calculate once' in mind, then I have to question whether any of the properties I'm storing in $this are well-advised (with a couple of exceptions, like the id of the form etc).

Let's take error messages as an example, as it stands, my methods for validating email addresses, URLs, checking for required fields having been filled in etc, all end with something like:



$this->errors[] = $this->getErrorMssgs('key name of relevant error message');

...so I'm creating an array of errors as a public property.

Is that standard? Or should I return the errors and build up an array outside the class in keeping with your above advice?


not to forget lazyness.

:D


getters (__get()), although they should be used carefully

Never heard of 'em! I'll read up. But given your cautions, I imagine I should be steering clear.

Gosh, this is all very helpful. Thank you, thank you, thank you.

Dormilich
04-03-2010, 09:00 PM
I take it from your "er" that this statement is to be taken with a pinch of salt. I'm not sure I understand. :confused: I don't suppose you could elaborate on 'required by the Interface'? When is something not?

making an Interface requires some thought of programme design (i.e. why do I do that this way), actually, I had to think for some time to find a reason for public properties.



Let's take error messages as an example, as it stands, my methods for validating email addresses, URLs, checking for required fields having been filled in etc, all end with something like:



$this->errors[] = $this->getErrorMssgs('key name of relevant error message');

...so I'm creating an array of errors as a public property.

Is that standard? Or should I return the errors and build up an array outside the class in keeping with your above advice?

that generally depends on how you handle errors. it is not wrong to collect class related errors in the class, but, on the other hand, what do you do with the errors afterwards? the answer to that should answer, what to do with the errors.

MattF
04-03-2010, 09:20 PM
It's all pretty much swings and roundabouts. There's so many different ways to do something it's shocking. :D



...so I'm creating an array of errors as a public property.

Is that standard? Or should I return the errors and build up an array outside the class in keeping with your above advice?


There's several ways that spring to mind. Call a public error function which outputs if no input is passed, have a private error function that the class calls at the end of processing, return the array to the calling script, (again public function), etc. You're putting way too much thought into complicating things which aren't. :D The whole point of classes, in my opinion, is the fact that you can make them do sod all or jump through hoops if you wish. There is no right/wrong, so to speak, as long as you stick to good coding standards.


p.s: Totally off topic, but guess which numpty just spent well over an hour trying to find a nonexistent error with a class that he'd shifted all of his general functions over to because he forgot to pass a reference to another class to it? :D



EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum