Proper Usage of a Controller
Moderator: General Moderators
-
SidewinderX
- Forum Contributor
- Posts: 407
- Joined: Fri Jul 16, 2004 9:04 pm
- Location: NY
Proper Usage of a Controller
In an MVC architecture, should the controller be responsible for passing POST/GET values to the model? From my understanding it _should_ but it doesn't seem practical since I am ending up with method headers and method calls miles long, and since they super globals it really is unnecessary to pass the values around.
$model->updateProject($_POST['project_id'], $_POST['title'], $_POST['start_month'], $_POST['start_day'], $_POST['start_year'], $_POST['end_month'], $_POST['end_day'], $_POST['end_year'], $_POST['description'], $_POST['visible']);
$model->updateProject($_POST['project_id'], $_POST['title'], $_POST['start_month'], $_POST['start_day'], $_POST['start_year'], $_POST['end_month'], $_POST['end_day'], $_POST['end_year'], $_POST['description'], $_POST['visible']);
-
alex.barylski
- DevNet Evangelist
- Posts: 6267
- Joined: Tue Dec 21, 2004 5:00 pm
- Location: Winnipeg
Re: Proper Usage of a Controller
Yes, at least I do, although I don't access GPC directly I use the response object.In an MVC architecture, should the controller be responsible for passing POST/GET values to the model? From my understanding it _should_ but it doesn't seem practical since I am ending up with method headers and method calls miles long, and since they super globals it really is unnecessary to pass the values around.
If your parameter counts are that long and you feel funny, that's your spider sense tingling letting you know it's maybe time to refactor the method into two or more.
I personally hate long parameter lists, but sometimes when dealing with models it's somewhat nessecary as a single table can easily have 10 or more fields. Tough call to make.
You could pass the parameters in via an associative array, which is an idea I've been toiling with for a while as it makes your code slightly more dynamic and easy to change. I'm pedantic about the order of parameters (integers first, strings, etc) so I frequently refactor the method signatures and using associative arrays would make the order irrelevant at the expense that documentation systems would no longer be able to generate a accurate API for your models, which is why I haven't done it yet!
The other option is to treat the model as an object, like a wrapper around a data mapper, and initialize the object with the params:
Code: Select all
$model = new Model();
$model->first_name = 'Alex';
$model->create();Re: Proper Usage of a Controller
Many or most frameworks will wrap the POST/GET values in a request object, which is passed to the controllers and models (possibly in a registry). Then you access the values by getters like $request->get('username')
- Jammerious
- Forum Commoner
- Posts: 59
- Joined: Sat Jun 27, 2009 11:30 am
- Location: Slovenia (EU)
Re: Proper Usage of a Controller
Me being a newbie, I'd try and make a general purpose function...SidewinderX wrote:$model->updateProject($_POST['project_id'], $_POST['title'], $_POST['start_month'], $_POST['start_day'], $_POST['start_year'], $_POST['end_month'], $_POST['end_day'], $_POST['end_year'], $_POST['description'], $_POST['visible']);
Code: Select all
public function get_post($posts){
if(!empty($posts)){
$ret = array();
foreach(explode($posts,",") as $post){
if(isset($_POST[$post]){
$ret[] = $_POST[$post];
}
}
return $ret;
}
return null;
} //you would need to add some more checks like to see if all of your requested fields were found etc.
// preping the data
$data = get_post("project_id,title,start_month");
//check if it is ok and stuff, like isset($data)
$model->updateProject($data); //maybe even the updateProject checks if all arguments were supplied...
Re: Proper Usage of a Controller
Instead of passing each field individually, just pass off the entire array.
Change the model to accept arrays and you have a much more maintainable interface
Code: Select all
class FooController {
public function barAction() {
$model = new Model();
$model -> updateProject($_POST);
...
}
}-
alex.barylski
- DevNet Evangelist
- Posts: 6267
- Joined: Tue Dec 21, 2004 5:00 pm
- Location: Winnipeg
Re: Proper Usage of a Controller
The only draw back now is your interface is arbitrary and impossible for documentors to generate doc for you.Instead of passing each field individually, just pass off the entire array.
While it's easy enough to print_r() the params inside the method to determine their names and guess at what they do, having that constantly in my face as a cnocrete interface while implementing software is personally important to me.
Yes you could document the structure/array emulating the fields something like:
Code: Select all
function doSomething($array /* $first_name, $last_name, $middle_name, $age, $sex */)
{
extract($array); // Emulate having local vars -- kinda anti-pattern-ish
}I guess that is why, in this case I prefer explicit programming practices, not to mention they are clear to even the most beginner of developers.
This is a topic of great interest to me as I constantly refactor my method signatures, more than anything else maybe, second to moving files around maybe.
Cheers,
Alex
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: Proper Usage of a Controller
It isn't arbitrary just because it is an array. It is implied that inside the method that accepts the array there would be checks for any/all necessary values -- with errors identified. Documentation is not only interfaces...PCSpectra wrote:The only draw back now is your interface is arbitrary and impossible for documentors to generate doc for you.
(#10850)
Re: Proper Usage of a Controller
It's not artbitray, it's just much less verbose. If the attributes of a Project object change (number or name) the interface doesn't have to. It's also much easier to handle (validate + filter) arrays than individual values. Since a method can only have one return value, if there are several error mesages it needs to be an array - better to be consistent with in/out parameters.The only draw back now is your interface is arbitrary and impossible for documentors to generate doc for you.
Documentating the individual parameters in such a case would only make it less maintainable as well, as any change to the parameters would require a change in the documentation (unless you forget and they go out of sync). My rule of thumb is - for more than 3 parameters, pass an array. Especially when many of the parameters might be optional - it's much worse to pass a bunch of nulls just to get to the parameter position you want in a method.
-
alex.barylski
- DevNet Evangelist
- Posts: 6267
- Joined: Tue Dec 21, 2004 5:00 pm
- Location: Winnipeg
Re: Proper Usage of a Controller
Assuming you performed validations in the model, certainly that would tell the developer what the expectations were, but when using a component or a method as a client developer, implemetation details are not typically what your after, it's the interface, which when passing an array is arbitrary.It is implied that inside the method that accepts the array there would be checks for any/all necessary values -- with errors identified.
The implementor is now responsible for making sure the API is documented, whereas, if you went with an explicit API, at least auto-documentors can do most of that for you.
I see benefits and nagatives to each approach, but having to manually maintain documentation, IMHO is the most boring and the most likely thing to be incorrect. Autodoc builders in this case are a god send.
-
alex.barylski
- DevNet Evangelist
- Posts: 6267
- Joined: Tue Dec 21, 2004 5:00 pm
- Location: Winnipeg
Re: Proper Usage of a Controller
I think I said pretty much the same thing in my first post.If the attributes of a Project object change (number or name) the interface doesn't have to. It's also much easier to handle (validate + filter) arrays than individual values
I'm a huge fan of consistency but this is probably a requirement due to your design choice when implementing models. I don't return error codes or messages in my models, I throw exceptions.Since a method can only have one return value, if there are several error mesages it needs to be an array - better to be consistent with in/out parameters.
It would be if you documented the parameters, I don't.Documentating the individual parameters in such a case would only make it less maintainable as well, as any change to the parameters would require a change in the documentation (unless you forget and they go out of sync).
My methods all use self describing parameters, so I only run the generator to build the API, no additional comments needed, something like:
Code: Select all
Person::createPerson($first_name, $last_name, $age, $city_id)p.s-arbitrary was the wrong word...perhaps 'less' explicit would be more appropriate...which possibly equates to less dynamic or more rigid. It's a toss up...explicit programming makes everything very clear, implied logic means you have more figuring to do but the pay off comes in maintenance.
It's the crux between dynamic and static programming principles that drive me crazy every day.
Cheers,
Alex
Re: Proper Usage of a Controller
Really? you consider an error in a submitted form to be an exceptional case? I usually just show the user an error message. It's pretty common practice to use an array of error messages for user feedback, never seen exceptions used in such a mannerI don't return error codes or messages in my models, I throw exceptions.
I might have missed it, but how can that be consistent with what you are saying now? if you specify all the data attributes of an entity directly in the interface, any change in those attributes will force the interface to change (and all uses of it in the application to change as well).I think I said pretty much the same thing in my first post.
That might seems nice for creating, but replace it with an update method (like the OP has asked about) and then you start passing off nulls to attributes you don't want to update. it starts looking messy, especially as the number of parameters grows (imagine a person entity with 10+ attributes)Code: Select all
Person::createPerson($first_name, $last_name, $age, $city_id)
Re: Proper Usage of a Controller
I agree with Pytrin here. if you have one or just a couple of fields/attributes, then it might make sense to set them each in the constructor. But as soon as you deal with more form values it gets messy. Forms can easily have dozens of fields. Are you really going to do
Code: Select all
Person::save($first_name, $last_name, $age, $city_id, $street, $streetnumber, $postcode, $interest, $favoritepet, $favoritecolor, $some, $more, $fields /* et cetc etc */)-
alex.barylski
- DevNet Evangelist
- Posts: 6267
- Joined: Tue Dec 21, 2004 5:00 pm
- Location: Winnipeg
Re: Proper Usage of a Controller
Yes sir, error messages and outgoing error messages/codes are so 1990'sReally? you consider an error in a submitted form to be an exceptional case? I usually just show the user an error message. It's pretty common practice to use an array of error messages for user feedback, never seen exceptions used in such a manner
Seriously though making the switch really cleaned up my code.
It's not consistent, why do I have to be consistent when playing devils adovcate with myself and anyone willing to listen?I might have missed it, but how can that be consistent with what you are saying now?
It's a give or take, drop fixed parameter support and say bye bye to autodocs...keep them in place and the problem of maintenance and refactoring becomes more of an issue.if you specify all the data attributes of an entity directly in the interface, any change in those attributes will force the interface to change (and all uses of it in the application to change as well).
I see little difference in updates to be honest, unless your trying to update a single field, in which case I would probably implement a updateField($name, $value) and keep the regular update() form for updating.That might seems nice for creating, but replace it with an update method (like the OP has asked about) and then you start passing off nulls to attributes you don't want to update. it starts looking messy, especially as the number of parameters grows (imagine a person entity with 10+ attributes)
-
alex.barylski
- DevNet Evangelist
- Posts: 6267
- Joined: Tue Dec 21, 2004 5:00 pm
- Location: Winnipeg
Re: Proper Usage of a Controller
No, of course not. I would probably break my form up into logic sections and have the model reflect those sections, something like:I agree with Pytrin here. if you have one or just a couple of fields/attributes, then it might make sense to set them each in the constructor. But as soon as you deal with more form values it gets messy. Forms can easily have dozens of fields. Are you really going to do
Code: Select all
updateAccountDetails()
updatePersonalDetails()
updateForumDetails()At the same time, I really like having the API for my entire application printed in front of me that I can refer to instantly and simultaneously while working on problems.
Re: Proper Usage of a Controller
well they're back.. and throwing exceptions over non-exceptional cases is so 2002Yes sir, error messages and outgoing error messages/codes are so 1990's
Personally, I don't use autodocs. I don't see much value in it, I document what needs documenting myself (ie, everything that isn't completely self explanatory). Documentation generated just from method definition is not very useful in my opinion (and any good IDE can do that by the way)say bye bye to autodocs
I'm not talking about one specific field. When updating often you are only updating a partial list of fields (for example, just the username and email). The array approach accepts a partial list of parameters the same as the full list, with the same interface as the create method. Much simpler to implement, maintain and modify.I see little difference in updates to be honest, unless your trying to update a single field, in which case I would probably implement a updateField($name, $value) and keep the regular update() form for updating.
How do you handle a variable number of arguments to update using your approach? do you create mock null variables to pass for the parameters that aren't available, or do you use error suppression? create a method for any partial combination of parameters? either solution sounds pretty bad