Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 0 down vote favorite

The client side version in JavaScript is here

This is the server side version of my control module. How does the structure look.

PostScript

Because this is a small prototype/app I have preferences or things I will consider when I will have a working prototype, they are - testing, try/catch/throw handling of errors, advanced abstractions / design patterns, defensive coding against programmer error / malicious users. Auto-Loading, optimization Will add these later if the project matures. 

<?php
function __autoload( $class_name ) { include 'class.' . $class_name . '.php'; }
$object_c = new CMachine(); 
$object_c->invoke();

class CMachine
{
    public function invoke()
    {
        $pipe = $this->getPipe();
        switch( $pipe['model'] )
        {
            case 'MUserNew': case 'MUserExist': 
                $model_object = new $pipe['model']( new SDB(), new SUniversals() , new SText( $pipe['page'] ), new SMessage() ); 
                $this->send( $model_object->invoke( $pipe['args'] ) );
                break;
            case 'MUserTry':
                $model_object = new $pipe['model']( new SDB(), new SText( $pipe['page'] ) );
                $test = $model_object->invoke( $pipe['args'] );
                $this->send( $test );
                break;
            case 'MUserAny': case 'MOrb': 
                $model_object = new $pipe['model']( new SDB() );
                $this->send( $model_object->invoke( $pipe['args'] ) );
                break;
            default:
                echo " -> Undefined Module Requested";
        }
    }
    private function send( $string_send )
    {
        echo "|A|" . $string_send;        
    }
    private function getPipe()
    {
        return json_decode( $_POST['pipe'], true );
    }
}
share|improve this question
Too lazy to write a full answer, but the first thing that jumps out at me is that it looks like your controller is actually a dispatcher and your models are actually controllers. – Corbin Jun 19 '12 at 0:18
...the models hold all my logic...once they are complete they all output a string with the results...the Controller calls the model ( a controller is suppose to call either the view or the model, but I have no views on my server )...and sends the result to the Browser... – Hiro Protagonist Jun 19 '12 at 16:26

1 Answer

up vote 2 down vote accepted

Would separate those includes a bit differently. I'd use a newline after each include then use a double new line when I wanted to section them off. All one line as you are doing, and not even spaces between them, could make someone think that you have only three includes. Speaking of includes. You are loading all of these classes that you aren't going to use. Take a look a lazy initialization. Assuiming you only need to use those classes once in this class, and that is in that switch statement, then you can just include the correct one via case and avoid the overhead of those other unneeded classes.

Would have to agree with Corbin about this looking more like a dispatcher/router. These are okay, just letting you know that what you have isn't a controller.

Would suggest moving the send() method out of the switch. It doesn't really appear to change much and is just causing you to rewrite code. Assign whatever paramters you need to in the switch and just call the method outside of it. To prevent the default case from doing this, since you mentioned wanting to use try/catch, I would throw an error there instead of using echo.

share|improve this answer
I called it router() at one point...how is a router different from a controller? The send() function provides error checking as all output must go through send() it is concatenated with an |A|, if this is not found on the client an alert is raised... – Hiro Protagonist Jun 19 '12 at 16:23
A router's or dispatcher's only job is to choose which model or controller to load and, in my opinion, which session information to pass to it. Such as GET, POST, etc... Think of it as an old fashioned switch board operator. She collects information she needs to forward your call. – mseancole Jun 19 '12 at 16:59
All the models I've seen show the controller doing this( see link...it shows the dispatcher handling only incoming requests w/ no ties to the model)...perhaps they are combined here...I actually need to remove the switch / case and load dynamically...I will update soon. – Hiro Protagonist Jun 19 '12 at 19:32
The good and the bad thing about all design patterns is that everyone adapts them to their own purposes. So the fact that you call a controller, what I call a router, and apparently what Corbin calls a dispatcher, only proves that these are loose terms used to describe an idea and not a strict standard. If you had never heard of a router or dispatcher before and I started explaining my switch board operator analogy, you'd immediately be able to identify it, but as what you think of as a controller. I may not agree on your definition of a controller, but its just a difference in opinions. – mseancole Jun 19 '12 at 20:11
show 8 more comments

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.