Password change/update -- model or controller
Moderator: General Moderators
-
alex.barylski
- DevNet Evangelist
- Posts: 6267
- Joined: Tue Dec 21, 2004 5:00 pm
- Location: Winnipeg
Password change/update -- model or controller
I've asked similar model or controller questions many times in the past and the answer or conclusions always seem to change.
I'm just now refactoring some models (the logic was getting nasty and validation needed centralizing) so I implemented a crude but effective validation library similar to that which is found in most frameworks which supoport filtering/validation chains.
Anyways, now I look at some model logic and think: Does this really belong here?
I'm resetting the password when an account is updated but only if the password field and confirmation password field are set and equal. This is a ocmmon task but I wonder where the conditionals should go. I'm seriously contemplating moving this logic into the controller as I can sort of see this being an Interface concern.
What says you? What are your counter arguments?
I'm just now refactoring some models (the logic was getting nasty and validation needed centralizing) so I implemented a crude but effective validation library similar to that which is found in most frameworks which supoport filtering/validation chains.
Anyways, now I look at some model logic and think: Does this really belong here?
I'm resetting the password when an account is updated but only if the password field and confirmation password field are set and equal. This is a ocmmon task but I wonder where the conditionals should go. I'm seriously contemplating moving this logic into the controller as I can sort of see this being an Interface concern.
What says you? What are your counter arguments?
Re: Password change/update -- model or controller
I would handle it in the model. With all insert/update operations, I usually pass the $_POST superglobal directly into a model method and let it decide the logic of handling it - which in my opinion is outside of the scope of the controller. Something like:
Code: Select all
class FooController {
public function barAction() {
if($this -> getRequest() -> isPost()) {
$foobar = new Foobar();
$foobar -> updateFoo($_POST);
}
//etc
}
}
-
alex.barylski
- DevNet Evangelist
- Posts: 6267
- Joined: Tue Dec 21, 2004 5:00 pm
- Location: Winnipeg
Re: Password change/update -- model or controller
Why do you feel it is outside the scope of the model?
Some interesting questions are raised by these type of questions: Even basic questions, like what is business logic/application logic/etc?
I'm interested in hearing other replies before I brain dump all that I am thinking...ideally what I'd like to see is a community disscussion of the pros and cons of each approach, and thus conclude a best practice as to whether it belongs in the model or controller.
So yea, why do you feel it's best in the model? Which BTW is where I have it now and where I've had it for ages.
I've refactored it into the controller and honestly...it's made the model a little easier and the controller a little more complex. As it stands I don't see much pro/con for either direction but I'm sure I iwll later after some more thought.
Cheers,
Alex
Some interesting questions are raised by these type of questions: Even basic questions, like what is business logic/application logic/etc?
I'm interested in hearing other replies before I brain dump all that I am thinking...ideally what I'd like to see is a community disscussion of the pros and cons of each approach, and thus conclude a best practice as to whether it belongs in the model or controller.
So yea, why do you feel it's best in the model? Which BTW is where I have it now and where I've had it for ages.
I've refactored it into the controller and honestly...it's made the model a little easier and the controller a little more complex. As it stands I don't see much pro/con for either direction but I'm sure I iwll later after some more thought.
Cheers,
Alex
Re: Password change/update -- model or controller
I said the opposite - it's outside of the controller scope.Why do you feel it is outside the scope of the model?
In my interpretation of MVC structure, controllers are responsible only for application flow (ie, provide control). They determine which model and/or view should be take action and provide them with the necessary parameters. If any application logic is not flow-control in nature, then it is outside of the controller scope. I am following the precept of fat models / thin controllers as I believe it is better from a design standpoint.
For this reason my model base class has built in validation / filtering methods (that are also aware of the data source structure, such as database fields), since I consider validation / filtering not to be flow-control logic. Responding to the results of such operations is in the scope of the controller if it involves application flow. For example:
Code: Select all
class FooController {
public function barAction() {
if($this -> getRequest() -> isPost()) {
$foobar = new Foobar();
$result = $foobar -> updateFoo($_POST);
if(is_array($result)) { // Error array returned
$this -> initView() -> errors = $result; //Pass errors into view
} else {
$this -> _redirect('/foo/index'); //Update successful, redirecting
}
}
//etc
}
}
- allspiritseve
- DevNet Resident
- Posts: 1174
- Joined: Thu Mar 06, 2008 8:23 am
- Location: Ann Arbor, MI (USA)
Re: Password change/update -- model or controller
No. In my opinion, user input validation belongs in the controller. Your model (specifically, mappers/gateways if you like to keep that stuff separate from your domain objects, like me) should only check to make sure it has enough information to insert/update in the database. It should throw an exception if it doesn't have enough information, because that means the developer did something wrong. Any validation that will display notices to the user on failure belongs in the controller.PCSpectra wrote:Anyways, now I look at some model logic and think: Does this really belong here?
Validation is something that depends on different contexts, which are outside the scope of the model. If you find you are repeating the same pattern of validation logic, those can be placed into a helper class that does the validation for the controller.
Pytrin and I have discussed this before... maybe some others could jump in and lend their opinion?
Re: Password change/update -- model or controller
I guess it's down to personal preference.
Having validation take place in the controller is basically describing the data structure of the model since validation has to know a lot about the properties of the data (data field names, types and limitations). In my opinion this breaks encapsulation - as the structure of the model data is prone to change (database fields get added/renamed/removed), and then you have to track down all the places you specified directly how this data is structured.
Having the validation in one place is much more maintainable. Putting it in a controller helper is not much help if you need one model to validate the data of another in one of its methods.
Having validation take place in the controller is basically describing the data structure of the model since validation has to know a lot about the properties of the data (data field names, types and limitations). In my opinion this breaks encapsulation - as the structure of the model data is prone to change (database fields get added/renamed/removed), and then you have to track down all the places you specified directly how this data is structured.
Having the validation in one place is much more maintainable. Putting it in a controller helper is not much help if you need one model to validate the data of another in one of its methods.
- allspiritseve
- DevNet Resident
- Posts: 1174
- Joined: Thu Mar 06, 2008 8:23 am
- Location: Ann Arbor, MI (USA)
Re: Password change/update -- model or controller
You know, it seems that way, but what you're really validating is the form, not the fields. The form fields could be named anything you want, and thus field names don't matter. The types and limitations you describe are generally form-specific, and could be stored in a number of ways in the database. The form could also be made up of several domain objects, or several tables, and the context of validation is outside of any one domain object's role.pytrin wrote:I guess it's down to personal preference.
Having validation take place in the controller is basically describing the data structure of the model since validation has to know a lot about the properties of the data (data field names, types and limitations). In my opinion this breaks encapsulation - as the structure of the model data is prone to change (database fields get added/renamed/removed), and then you have to track down all the places you specified directly how this data is structured.
Having the validation in one place is much more maintainable. Putting it in a controller helper is not much help if you need one model to validate the data of another in one of its methods.
Encapsulation tends to work best for business logic. Because validation is a cross-cutting concern, it should be located with other cross cutting concerns-- in the controller, with authentication and authorization. Keeping it in the model clutters up your domain objects, and breaks the single responsibility principle.
Could you share a use-case where keeping validation in the model makes maintenance easier? It seems to me any changes in the domain object's properties are going to ripple out into the db, the view (form), and the controller anyways. If all you're doing is changing validation, it shouldn't really matter which place you have your validation, since its all in once place.
I'm not sure what you mean by the last sentence.
Re: Password change/update -- model or controller
That's a matter of perspective. From my vantage point, before the model can touch its data source (database mostly) it has to be sure the data it will send is both filtered and validated. Input doesn't arrive only from forms, it can be invoked by a public API/REST service and other sources. The model is only concerned with validating data pertaining to it, it doesn't validate forms per-se. Such validation can be left to the controller if it does not pertain to the model.what you're really validating is the form, not the fields
I'm not sure what you mean by cross-cutting, but encapsulation is a basic OO paradigm and is not limited to business logic at all. Again, I'm not talking about form validation - only data validation that pertains to the model. Sometimes those two coincide but not always.Encapsulation tends to work best for business logic. Because validation is a cross-cutting concern
My models map to database tables almost all the time. When a database table schema changes, only the model has to change to reflect it in the application code. Model data can be changed from several different controller actions, but it matters not since the model is the only one that knows about the data structure.Could you share a use-case where keeping validation in the model makes maintenance easier?
- allspiritseve
- DevNet Resident
- Posts: 1174
- Joined: Thu Mar 06, 2008 8:23 am
- Location: Ann Arbor, MI (USA)
Re: Password change/update -- model or controller
Sure, data can come from other sources... that shouldn't change things. Either use the same controller for each input source, or (if the validation rules are the same across input types) use a helper, as I mentioned before.pytrin wrote:That's a matter of perspective. From my vantage point, before the model can touch its data source (database mostly) it has to be sure the data it will send is both filtered and validated. Input doesn't arrive only from forms, it can be invoked by a public API/REST service and other sources. The model is only concerned with validating data pertaining to it, it doesn't validate forms per-se. Such validation can be left to the controller if it does not pertain to the model.
Why should the model care whether the data is filtered and validated? As long as that data can be inserted/updated in the database, and used to perform business logic, who cares if a user's first AND last name are requred? Who cares if a headline has to be less than 100 characters in order to not take up too much space on the page? As long as the model can do its job, it shouldn't complain. It also shouldn't have to worry about sending messages to the controller telling the controller why a certain form field wasn't filled out correctly.
I guess I spoke too hastily regarding encapsulation... I meant that in the model, you want changes to business logic to be localized so they don't ripple out into the application. Changes to data affect all parts of the application, hence cross-cutting. Data validation pertains to the input source, not the model.pytrin wrote:I'm not sure what you mean by cross-cutting, but encapsulation is a basic OO paradigm and is not limited to business logic at all. Again, I'm not talking about form validation - only data validation that pertains to the model. Sometimes those two coincide but not always.
The only way to minimize schema impact is to have a really strong ORM layer and a rich domain model. In cases where domain objects map 1:1 to database changes, schema changes are going to affect everything. How do users interact with your model, if neither your controller or your view is aware of what data the domain objects contain?pytrin wrote:My models map to database tables almost all the time. When a database table schema changes, only the model has to change to reflect it in the application code. Model data can be changed from several different controller actions, but it matters not since the model is the only one that knows about the data structure.
-
alex.barylski
- DevNet Evangelist
- Posts: 6267
- Joined: Tue Dec 21, 2004 5:00 pm
- Location: Winnipeg
Re: Password change/update -- model or controller
Thats a good argument.Your model (specifically, mappers/gateways if you like to keep that stuff separate from your domain objects, like me) should only check to make sure it has enough information to insert/update in the database.
Good point.It should throw an exception if it doesn't have enough information, because that means the developer did something wrong. Any validation that will display notices to the user on failure belongs in the controller.
Yet another.Validation is something that depends on different contexts, which are outside the scope of the model.
That is one of the counter arguments I have against putting validation in the model. Keep in mind that a model will centralize that logic so now regardless of who calls that function will get those exceptions or error codes. For instance, what happens on a REST request? Do you use that helper validation class again before invoking the model? If you just kept the validation in the model, this wouldn't be a concern, so I guess it depends on how important data integrity is to your application. It's it's mission critical, validation should absolutely be stored in the model so you gaurantee data is properly validated before passed to RDBMS.If you find you are repeating the same pattern of validation logic, those can be placed into a helper class that does the validation for the controller.
On the other hand, if somoene uses a REST API to insert data into your application, they should probablt be responsible for ensuring data integrity, whereas the model should be focused on business logic and the data access layer concerned with escaping and other security measures pertinent to data store.
If you consider that a majority of validation can be done on client side and most people would consider that "controller" logic when done in that context but you directly utilize the UI. Why is it when validation is done on the server side, it's automatically assumed to be "model" logic or business logic?Having validation take place in the controller is basically describing the data structure of the model since validation has to know a lot about the properties of the data (data field names, types and limitations).
I agree. It's similar to the REST API problem I pointed out above. If your data input changes, you would need to update the model helper or both your standartd controller and REST scripts. If everything was done in the model using helper methods it would be a lot easier to maintain. And the model does change a lot as requirements come and go.Having the validation in one place is much more maintainable. Putting it in a controller helper is not much help if you need one model to validate the data of another in one of its methods.
Touche.Encapsulation tends to work best for business logic. Because validation is a cross-cutting concern, it should be located with other cross cutting concerns-- in the controller, with authentication and authorization. Keeping it in the model clutters up your domain objects, and breaks the single responsibility principle.
Good point.it can be invoked by a public API/REST service and other sources. The model is only concerned with validating data pertaining to it, it doesn't validate forms per-se. Such validation can be left to the controller if it does not pertain to the model
There are many who favour/argue the fact one should have very trivial/simple controllers, with the brunt of the work being done in the view/template and model/dal.
As it stands right now, my controllers are quite simple, so simple in fact, they almost seem pointless. My example of password validation is interesting, because it's not strictly tied to business logic, so much as it is to application logic. The model would need both the original password and the confirmation password in order to perform the comparison.
Why would you pass a parameter to a model that isn't even explicitly needed by the model (ie: Confirmation password)? To me this screams controller logic or something outside the model domain at least.
Re: Password change/update -- model or controller
Validation logic can be core to your business objects, or apply to only certain contexts. So there is no answer for all situations. In an ideal world ( in my opinion ) model state should be immutably valid ( for instance a user model should not allow anything that is not a timepoint or a subclass of timepoint to be passed in for the birthdate attribute, the model's client shouldn't have to know about a method called ->isValid(), the model should actively only accept correct data ). Then your controllers / form controllers can augment that validation logic for finer grained input control. There's a long thread in the skeleton forum about this topic that came to similar conclusions. For instance since PHP is loosely typed and you can pass an int as a string, if integers are invalid for a certain context that should be checked by the form, before the model ever gets the data. Trying to assign data that is simply invalid for the model is an exception and a programming error
-
alex.barylski
- DevNet Evangelist
- Posts: 6267
- Joined: Tue Dec 21, 2004 5:00 pm
- Location: Winnipeg
Re: Password change/update -- model or controller
I'm gld I started thinking about this...I've refactored about 1000 lines of code already and really cleaned my models up big time...so much so I'm almost thinking I might stay with my current approach as opposed to opting for a OR/M solution -- although I still need to eventually implement one.
I still haven't decided whether to keep my validation in the model or controller or helpers...what I have done is removed all conditional logic from all models and controllers...I will print off all my source files and analyze the pros and cons and eventually come to a conclusion that best fits my development style, application architecture, etc...
Keep the disscussion going by all means...the more arguments I have in either direction will only serve to help me (and the community as a whole) make a better decision.
Cheers,
Alex
I still haven't decided whether to keep my validation in the model or controller or helpers...what I have done is removed all conditional logic from all models and controllers...I will print off all my source files and analyze the pros and cons and eventually come to a conclusion that best fits my development style, application architecture, etc...
Keep the disscussion going by all means...the more arguments I have in either direction will only serve to help me (and the community as a whole) make a better decision.
Cheers,
Alex
- allspiritseve
- DevNet Resident
- Posts: 1174
- Joined: Thu Mar 06, 2008 8:23 am
- Location: Ann Arbor, MI (USA)
Re: Password change/update -- model or controller
What, if I may ask, is your current approach?PCSpectra wrote:I'm gld I started thinking about this...I've refactored about 1000 lines of code already and really cleaned my models up big time...so much so I'm almost thinking I might stay with my current approach as opposed to opting for a OR/M solution -- although I still need to eventually implement one.
-
alex.barylski
- DevNet Evangelist
- Posts: 6267
- Joined: Tue Dec 21, 2004 5:00 pm
- Location: Winnipeg
Re: Password change/update -- model or controller
Nothing fancy I just use a phrasebook pattern approach to remove the SQL from the model code. Each SQL statement is stored in a INI file as a PDO prepared statement.
In theory I could easily swap out MySQL with MSSQL or any other database supported by PDO.
I favoured this approach over AdoDB or heavy weight ORM solutions simply for speed and total flexibility offered by writing custom SQL statements. The one single downside is the static nature of a prepared statement, especially stored in phrasebook.
For advanced dynamic queries like search engine, etc you have two options:
1. Use dynamic placeholders with in the SQL string, replace them with PHP at runtime as needed. Risky as you could inadvertently introduce security issues -- although I've been extremely careful it's still possible.
2. Write the SQL statements one for each condition. While this works for trivial dynamic queries you can see how it would quickly become unmanagable, even with SQL stored nicely in an SQL file with full syntax hilighting, etc...
Cheers,
Alex
In theory I could easily swap out MySQL with MSSQL or any other database supported by PDO.
I favoured this approach over AdoDB or heavy weight ORM solutions simply for speed and total flexibility offered by writing custom SQL statements. The one single downside is the static nature of a prepared statement, especially stored in phrasebook.
For advanced dynamic queries like search engine, etc you have two options:
1. Use dynamic placeholders with in the SQL string, replace them with PHP at runtime as needed. Risky as you could inadvertently introduce security issues -- although I've been extremely careful it's still possible.
2. Write the SQL statements one for each condition. While this works for trivial dynamic queries you can see how it would quickly become unmanagable, even with SQL stored nicely in an SQL file with full syntax hilighting, etc...
Cheers,
Alex