Looking for a supremely good example of class that should never be instantiated

sneakyimp

I'm currently in the midst of a debate in the CodeIgniter forums about their model-loading functionality. I am admittedly being a total pedant but wanted to point out that their method of loading model classes, which is ostensibly designed for the sake of convenience, is silly because it automatically instantiates an instance of that class and attaches it to the current controller. I.e.:

// this is a controller method:
  public function index() {
    // i want to load my class:
    $this->load->model("Myclass");
    // this unfortunately instantiates my class:
    $this->Myclass->foo();
  }

Obviously, this is not always ideal. One can supply parameters to the loader which are handed to the Myclass constructor, but this obviously means the constructor must take a single parameter which might force redesign of existing classes.

What I'm looking for is the obvious you-should-never-instantiate-this-class example. I've already mentioned classes with all-static classes but I'm still encountering resistance.

NogDog

Caveat: I was too lazy to follow the discussion link.

Not sure if this is what you're getting at, but back when I did some stuff with CI (several years ago now), for models that were only needed by one/some method(s) in a controller, I would put the load->model call(s) in that method, not in the controller's constructor. Is that the sort of thing you're talking about?

sneakyimp

NogDog;11044533 wrote:
Not sure if this is what you're getting at, but back when I did some stuff with CI (several years ago now), for models that were only needed by one/some method(s) in a controller, I would put the load->model call(s) in that method, not in the controller's constructor. Is that the sort of thing you're talking about?

Sort of. Yes of course it makes perfect sense to only load a model (or include a class) when it is needed. That's a matter of optimization. The issue I'm all fired up about is that it seems stupid to me to automatically instantiate a model class when you load it. CI does allow you to send one parameter with the load command, but this automatic instantiation seems just plain silly to me. What if I want 3 instances of the object? Or none (because it's a class with all static methods) ?

NogDog

I must admit the possibility has never entered my mind, but then I very rarely make use of static class variables and even less with static methods (which in CI begin to smell more like "helper" functions to me?). I can see where it might occasionally be useful to have multiple instances of a model, but then that might be pointing to the need for a container class (e.g. Composite pattern)?

sneakyimp

NogDog;11044537 wrote:
I must admit the possibility has never entered my mind, but then I very rarely make use of static class variables and even less with static methods (which in CI begin to smell more like "helper" functions to me?). I can see where it might occasionally be useful to have multiple instances of a model, but then that might be pointing to the need for a container class (e.g. Composite pattern)?

I've only had rare occasion to use static class vars (a list of values for a Registry object comes to mind) but I have on many occasions seen opportunity to use static functions. A Validator class to encapsulate all kinds of input validation was one. Other ones include functions to consistently derive a string or value from other values in a consistent way. Attaching such functions to a particular class is really helpful to:
a) avoid name collisions with other people's function names -- very helpful in a complex project where you aren't familiar with other folks' code
b) grouping of functions - keep functions close to the object/class that it typically operates on
c) autocomplete - you don't need to remember specific function names, you just remember the class and start typing it and your IDE brings up the list of attached static methods.

Could you elaborate on the Composite pattern? A link perhaps?

NogDog

Here are the first couple promising-looking links I found (the 2nd goes into a lot more depth -- and I'd guess you'll like better 😉 ):
http://sourcemaking.com/design_patterns/Composite/php
http://www.php5dp.com/the-composite-design-pattern-in-php-part-i-from-conceptual-to-practical/

Derokorian

Can't say I have an answer for you, but that maybe comes from the fact that I have a differing opinion about what models are for. I believe models are for interacting with data stores. I use a model to read from the db and change the data to a non-db format if necessary. I use it to write to a db, validating and modifying it for this specific store (not necessarily this specific data, I may store it in 2 places and require different formats or rules to be followed), instead the controller will validate the web request meets the requirements of this API end point. I use models to send emails, but not to compose the emails or the list of recipients.

Based on your post in the discussion you linked, I would say we use models differently. I would say that the way I make use of the Model layer, requires an instance whenever I use it. I tried to rap my brain for a reason not to need an instance, and I came up a bit empty.

I agree, however, that you should be able to have multiple instances, and moreover be able to pass in an arbitrary number of arguments.

Derokorian

Oh one more thing...

includebeer Wrote:

You can pass arguments to the loader class in the second parameter:
$params (array) – Optional array of parameters to pass to the loaded library’s constructor

I'm aware you can pass one parameter to your constructor using this method and I still say it's silly. Am I supposed to rewrite all of my classes such that their constructor takes only one array parameter? What is the rationale for this subversion of PHP's power by limiting my constructors to one parameter? Silly, silly, silly. Also, what about static classes that are never supposed to be instantiated?

This is an array of parameters to be passed to the constructor, much the same way that [man]call_user_func_array[/man] passes many arguments to a function, but only one argument that is an array of those arguments to the call function.

function myFunc($var1, $var2, $var3) {
	$args = func_get_args();
    printf("Number of arguments: %d\nCalled with: %s\n", func_num_args(), print_r($args, true));
}

call_user_func_array('myFunc', array(1,2,3,4,5));

/**
Number of arguments: 5
Called with: Array
(
    [0] => 1
    [1] => 2
    [2] => 3
    [3] => 4
    [4] => 5
)
*/

sneakyimp

NogDog;11044549 wrote:
Here are the first couple promising-looking links I found (the 2nd goes into a lot more depth -- and I'd guess you'll like better 😉 ):
http://sourcemaking.com/design_patterns/Composite/php
http://www.php5dp.com/the-composite-design-pattern-in-php-part-i-from-conceptual-to-practical/

Thank you for the links. Composite reminds me of collections in VB. I still think there are situations where having two distinct instances of an object is preferable to some composite or collection object. For example, if you have two players in a game. Seems to me that having to refer to some aggregate object first would really just make your code more verbose:

$p1 = Player::fetch($db, 'WilliamTheConqueror');
$p2 = Player::fetch($db, 'KingHarold');

$p1->attack();
$p2->flee();

In this case, the objects, although similar, are doing drastically different things that are related. A composite object would simply get in the way.

sneakyimp

Thanks for your posts, Derokorian...going to respond in a second. In the meantime:

Derokorian;11044555 wrote:
Oh one more thing...
This is an array of parameters to be passed to the constructor, much the same way that [man]call_user_func_array[/man] passes many arguments to a function, but only one argument that is an array of those arguments to the call function...

If only this were true. In fact, CI doesn't work that way. If your controller looks like this:

class Test extends EREP_Controller {
    public function index()
    {
    	$args = array("ONE", "TWO", "THREE");
        $this->load->library("Test_library", $args);
    }
}

And Test_library looks like this:

class Test_library {
	public function __construct($p1, $p2, $p3) {
		var_dump($p1);
		echo "\n";
		var_dump($p2);
		echo "\n";
		var_dump($p3);
		echo "\n";

}
}

Then you get a bunch of warnings because CI will pass the array $args as a single parameter to the constructor of your library and $p2 and $p3 will not receive any values:

Warning: Missing argument 2 for Test_library::__construct()
Warning: Missing argument 3 for Test_library::__construct()
array(3) { [0]=> string(3) "ONE" [1]=> string(3) "TWO" [2]=> string(5) "THREE" } 
Notice: Undefined variable: p2
NULL
Notice: Undefined variable: p3
NULL

If you have any libraries whose constructors require multiple parameters, then you would need to rewrite (or extend) them to receive a single parameter that contains all the necessary values.

NogDog

As far as the 2-player example goes...

$this->load->model('Player', 'player1');
$this->load->model('Player', 'player2');
$this->player1->flee();
$this->player2->attack();

All that being said, it's still a fact that frameworks, by their nature, are easier to use for code that fits their way of doing things, versus trying to make them work the way you want them to. If you find yourself wrestling with a framework to match your preferred coding style and techniques, then that framework might not be for you -- or more specifically for the type of application you are trying to build. (I have seen some [rather ivory tower-ish] rants about why MVC doesn't even make sense in a HTTP asynchronous application, so who knows?)

sneakyimp

Derokorian;11044553 wrote:
Can't say I have an answer for you, but that maybe comes from the fact that I have a differing opinion about what models are for. I believe models are for interacting with data stores. I use a model to read from the db and change the data to a non-db format if necessary. I use it to write to a db, validating and modifying it for this specific store (not necessarily this specific data, I may store it in 2 places and require different formats or rules to be followed),

This all sounds great to me. I'm not sure what I might have expressed that would cause us to think differently about any of these points, although I do have a vague feeling that my idea of what belongs in MODEL may be considerably larger than your ideas. I've hard a lot of talk about 'fat models, skinny controllers' being ideal. If you could elaborate, I'd appreciate it. I'm hoping to learn here.

Derokorian;11044553 wrote:
instead the controller will validate the web request meets the requirements of this API end point.

That sounds reasonable I suppose. On the other hand, the question "what is valid data for this data object?" seems best addressed to the model which must enforce data integrity and, as I understand it, "business logic". A similar question that seems entirely specific to the Model might be "can user X change data point Y." In my reckoning, whether or not the request "meets the requirements of the API end point" would be a question in which the controller's role is limited. It would analyse the incoming request, check a few authentication params (all provided by some model of course), make sure the request was GET or POST as appropriate, perhaps check encoding, and then route any input data to the appropriate model which would do the heavy lifting. The Model would then return the data which the controller would dutifully digest and route to the appropriate appropriate view.

The problem is that all these descriptions I'm providing are vague and don't really clear much up 🙁

Derokorian;11044553 wrote:
I use models to send emails, but not to compose the emails or the list of recipients.

It seems pretty apparent to me that a controller has some responsbility in formulating communications to a user -- taking care not to expose sensitive error conditions to potentially malicious users, for instance -- however, other types of communication seem like none of the controller's business. For instance, say a newsletter must be sent to all customers about a sale. This event might be triggered by a controller interpreting a user action, but the actual work of the email sending should probably be delegated to a library which interacts with the model to collect all the customer's email addresses. The whole job should furthermore be performed asynchronously rather than forcing the user to wait 24 hours for 100,000 emails to be sent.

Derokorian;11044553 wrote:
Based on your post in the discussion you linked, I would say we use models differently. I would say that the way I make use of the Model layer, requires an instance whenever I use it. I tried to rap my brain for a reason not to need an instance, and I came up a bit empty.

OK I am having trouble with this too -- thus my post posing the question. How about the Factory pattern? Or some other singleton class?
http://www.phptherightway.com/pages/Design-Patterns.html

Derokorian;11044553 wrote:
I agree, however, that you should be able to have multiple instances, and moreover be able to pass in an arbitrary number of arguments.

This is also for NogDog. I'm not 100% certain yet, but I believe that one cannot pass any parameters to a model in CI when loading a model like this:

class Mycont extends CI_Controller {
  public function index() {
    // this instantiates the Model class Mymodel, but there is no way to supply params
    $this->load->model("Mymodel");
  }
}

sneakyimp

NogDog;11044561 wrote:
As far as the 2-player example goes...
$this->load->model('Player', 'player1');
$this->load->model('Player', 'player2');
$this->player1->flee();
$this->player2->attack();

One problem with that code is that you need some additional lines of code to actually go and fetch the appropriate data for player1 and player2. You cannot provide any parameters to the models via the loader. This both restrict one's class designs by limiting you to constructors with no parameters and requires additional lines of code to retrieve valid model contents. It has the additional issue that it requires a reference to $this to refer to the players. And finally, it may expose the player objects loaded to a broader scope than one wants.

NogDog;11044561 wrote:
All that being said, it's still a fact that frameworks, by their nature, are easier to use for code that fits their way of doing things, versus trying to make them work the way you want them to. If you find yourself wrestling with a framework to match your preferred coding style and techniques, then that framework might not be for you -- or more specifically for the type of application you are trying to build. (I have seen some [rather ivory tower-ish] rants about why MVC doesn't even make sense in a HTTP asynchronous application, so who knows?)

Well said! I am getting the feeling I'm coming off a bit like the ADD kid in your classes who just won't shut up and learn the lesson, but I sort of can't help asking why things are done this way in CodeIgniter. So far I feel like the answer is "shut up and learn to do it the orthodox way." One of the things I like about CI is that it doesn't seem constrictive and the flow of code execution seems pretty transparent to me. I suppose I need to change my tack and start asking why its advantageous to do it as they have rather than pointing out the problems with it.

Derokorian

sneakyimp;11044559 wrote:
If only this were true. In fact, CI doesn't work that way.
If you have any libraries whose constructors require multiple parameters, then you would need to rewrite (or extend) them to receive a single parameter that contains all the necessary values.

Well I'll be a monkey's uncle. That's quite stupid, and the documentation is very misleading to me, who did not test it.... I'm saddened by this knowledge!

NogDog

sneakyimp;11044565 wrote:
One problem with that code is that you need some additional lines of code to actually go and fetch the appropriate data for player1 and player2....

If that is so (I don't recall either way), then you'd have to pass those parameters into a (non-constructor) method of that model, either in a separate command, or possibly by method chaining if you have that method return the object instance, perhaps -- in either case something you'd have to do anyway, regardless of how many model objects are being instantiated, right?

sneakyimp

Derokorian;11044577 wrote:
Well I'll be a monkey's uncle. That's quite stupid, and the documentation is very misleading to me, who did not test it.... I'm saddened by this knowledge!

As am I. CI doesn't force you to use their loader to locate your libraries at all, but it just seems odd to me that the authors would have worked in something that is so constraining as a fundamental aspect of their library loader.

NogDog;11044579 wrote:
If that is so (I don't recall either way), then you'd have to pass those parameters into a (non-constructor) method of that model, either in a separate command, or possibly by method chaining if you have that method return the object instance, perhaps -- in either case something you'd have to do anyway, regardless of how many model objects are being instantiated, right?

Sadly, I believe so. I'm left wondering what real advantage this model-loading functionality provides. I have resorted to creating my own autoloader and instantiating my objects the usual way. I see no advantage at all to using the default CI method for loading models or libraries.

PharaohTools

Pharaoh tools got really good solutions to softwares and stuff. You should try them out and check their website on www.pharaohtools.com really a huge help!

dalecosp

Heheh, great choice of place to spam, there, "Pharoah" ... "a class that should never be instantiated" ... sounds like a perfect place 😉 😉