patternphpModerate
MVC Controller Class
Viewed 0 times
classmvccontroller
Problem
Here is my controller class for my MVC pattern with all the includes removed. From what I know of the MVC pattern it is good to have a small controller class. Mine is just a collection of public static functions which accept requests and send data to the client.
Regarding
I have a
Snippet 1
```
add();
break;
case 'ControlBookmark_delete':
$control_bookmark_instance = new ControlBookmark('remove');
$control_bookmark_instance->delete();
break;
case 'ControlTweet_add':
$control_tweet_instance = new ControlTweet('remove');
$control_tweet_instance->add();
break;
case 'ControlSignIn':
$control_signin_instance = new ControlSignIn('remove');
$control_signin_instance->invoke();
break;
case 'ControlSignUp':
new ControlSignUp();
break;
case 'ControlTryIt':
new ControlTryIt();
break;
case 'ControlSignOut':
Session::finish();
new ControlSignOut();
break;
default:
throw new Exception('Invalid ajax_type');
}
}
public static function reload() // Because reload may be triggered by File Upload (second control path)
{
if (session_id() == "")
{
Session::start();
}
Regarding
send(): I'm just wrapping the echo command for sending text from an Ajax call. This is for consistency and to have all text exiting the same point in code.I have a
ControlEntry (Snippet 2) which help decouple super globals and direct entry into the Control, so the two kind of work in tandem.Pull_Pic just pulls a path and inserts it into HTML for display.Snippet 1
```
add();
break;
case 'ControlBookmark_delete':
$control_bookmark_instance = new ControlBookmark('remove');
$control_bookmark_instance->delete();
break;
case 'ControlTweet_add':
$control_tweet_instance = new ControlTweet('remove');
$control_tweet_instance->add();
break;
case 'ControlSignIn':
$control_signin_instance = new ControlSignIn('remove');
$control_signin_instance->invoke();
break;
case 'ControlSignUp':
new ControlSignUp();
break;
case 'ControlTryIt':
new ControlTryIt();
break;
case 'ControlSignOut':
Session::finish();
new ControlSignOut();
break;
default:
throw new Exception('Invalid ajax_type');
}
}
public static function reload() // Because reload may be triggered by File Upload (second control path)
{
if (session_id() == "")
{
Session::start();
}
Solution
It looks like you are trying to write object oriented code, however this is completely procedural. Remove static everywhere and use dependency injection if you want to make this OO.
Static code is hard to test. It is the equivalent of a namespaced function.
Even your new objects that you create are the equivalent of calling a function.
I'd recommend one of the following:
Some people may disagree that I would suggest moving away from object oriented code, but your code is so far away from object oriented that it makes sense to me.
Response to the Edit
It does look a bit better now. It would be a good experience for you to unit test this with PHPUnit. I think you would find it very hard with the
One comment on the edited version:
Static code is hard to test. It is the equivalent of a namespaced function.
Even your new objects that you create are the equivalent of calling a function.
case 'ControlBookmark_add':
new ControlBookmark('add');
break;I'd recommend one of the following:
- Spend a long while learning how to write real object oriented code.
- Give up and convert to procedural code. Skipping all of the wasted classes and
new's
Some people may disagree that I would suggest moving away from object oriented code, but your code is so far away from object oriented that it makes sense to me.
Response to the Edit
It does look a bit better now. It would be a good experience for you to unit test this with PHPUnit. I think you would find it very hard with the
static and new. That would give you a motivation to change it further. However, I'm guessing the testing this code will receive will be based on seeing the page on the browser, which is okay for a lot of people.One comment on the edited version:
// Do you really need the 'remove' parameter to your constructor?
// Especially when you call the add method on it?
// Also, objects aren't that special I wouldn't bother with _instance.
$control_bookmark = new ControlBookmark();Code Snippets
case 'ControlBookmark_add':
new ControlBookmark('add');
break;// Do you really need the 'remove' parameter to your constructor?
// Especially when you call the add method on it?
// Also, objects aren't that special I wouldn't bother with _instance.
$control_bookmark = new ControlBookmark();Context
StackExchange Code Review Q#7676, answer score: 10
Revisions (0)
No revisions yet.