At first pass, my ErrorCollection.cfc could only accept error messages for data validation issues discovered in the Service Layer of the application. This was based on the thought that all validation would need to go in the Service layer as this was the master mind behind the business logic of the application. Well, German Buela just pointed me to a good post that really rocked that assumption to the ground.
At first I was thinking that the only validation that mattered was the business logic validation, but this is not true. The user interface has validation to it as well. One of the examples used in the above link included password verification - the common approach in which the user has to enter their password twice to help ensure that there are no spelling mistakes. When the form is submitted, checking that those two form fields are the same (that the password has no spelling mistakes) is not really a business logic validation item. This two-password-entry scenario is simply a construct of a user interface that is providing a better user experience; it has nothing to do with the business logic of the application at all. In fact, if the interface provided only one password input, the business logic would remain unchanged.
So, where does the validation between the two password fields have to occur? In the controller logic as part of the form data validation and form processing. This raises two hugely important points:
- Not all validation can be done in the service layer; some of it can be and must be done in the controller layer.
- The controller needs access to the ErrorCollection.cfc ColdFusion component BEFORE any calls to the Save() method located in the Service Layer.
Here's my logic on #2: Let's say we have a form where the user is entering their password. All the form has is the two password fields (one real one and one for spelling validation). That's it. Now, when that form gets submitted to the server, the controller cannot rely on a ErrorCollection.cfc to come back from the Save() method. If we did that, and the first password field was "valid", then the password would get sent to the database before we checked for matching field values. Therefore, it is essential that the field value symmetry must be checked before any calls to Save(). Since we want to have a unified object for error collection, the controller layer either needs to instantiate an ErrorCollection.cfc outside of the service layer, or it needs to be able to call a non-committing Validate() method in the service layer.
I think what we are seeing here is that we should come up with a way to merge error collection objects. Like ColdFusion's StructAppend(), we need a way to take error collection from multiple places and concatenate them in such a way that when passed to the View, all errors will centrally located and the View won't need to care about where they are coming from.
What we are also seeing is that the ErrorCollection.cfc needs to be able to take random messages that aren't necessarily tied to any service layer validation. Messages like, "Your passwords do not match", which has nothing to do with the service layer, must be accepted by the error collection object.
This whole revelation makes me lean towards the idea of having the controller layer call a Validate() method outside of any Save() method. Of course that would mean that the Save() method would have to perform duplicate validation logic. This would be a violation of the DRY principle (don't repeat yourself), but as Henry pointed out in a previous comment, it is OK to sacrifice some performance for robustness.
Looking For A New Job?
- Software Engineer at Fairfax County Public Schools
- Senior Coldfusion Engineer - Can work remotely at Eventsquid LLC
- Hiring Senior Cold Fusion Engineers at Giva, Inc.
- Web Developer at Association for Computing Machinery
- SQL/ColdFusion Database Developer at NADAP
You may be starting to build your own validation framework as a sort of cross-cutting ("vertical") layer. Sounds like a big deal but it shouldn't be much more than a bag of messages where any layer can add something. The controller would take care of initializing it and handling it (if it finds it didn't end up empty, it means something happened and the user must be notified). I have worked with this approach before. Another question is how much do you want your layers to depend on it--this definitely creates a dependency. Another approach is to have your layers simply throw exceptions and let the controller handle them and display messages. It's cleaner to me, though you lose the reporting of multiple same-layer issues (I can live with that).
Regarding the DRY principle, it's more about code than execution. I mean, having separate chunks of code that do the same thing. Having a single chunk of code execute redundantly is not something you want to ignore but it's probably less of an issue.
First off, good point about the DRY stuff. As I get more into this, I don't always understand the principles fully. But, as you are saying, I can see the difference between duplicating code logic and calling encapsulated code from multiple places. Thanks for giving me a little reality nudge there.
As for the cross-layer dependence on the error collection object, I am comfortable with this, at least at the moment. I am very uncomfortable with the idea of throwing and catching exceptions as part of data validation; this can only lead to a horrible user experience. Imagine if there was a form with 10 required fields, none of which were entered - based on the returned error messages, it would take the user 10 form submissions to be told about all the reasons that the form could not be submitted.
Taking that scenario into account, I am comfortable about the cross layer dependency as the trade off to a better user experience.
And, as you are saying, the error collections is just a "bag" of messages. I am not worried about that. The problem is that if random messages can be added, it becomes more complicated to see is each there is a message for each validation error. Of course, that really only becomes an issue if I was planning to handle SQL exceptions... which might seem less and less appropriate.
i've been thinking about this exact topic recently for the exception/error handling portion of my new site script.
i'm still at the plannign stage but as best i can see it you would typically need to return two types of errors to the user... generic errors (e.g. stuff that crops up when building the page like page level authentication [you do not have access to this page]) and specific errors (from functionality that the user is expecting to do something for them like a login form validation failed [invalid username] or displaying a list of products from a search [no products found]).
generic ones typically appear at the top of the page perhaps more prominently, while specific ones may be less prominent and would be output in the section of the page related to where the error came from (e.g. next to the relevant fields in form ).
i'm using the model glue framework and the best way i've thought of so far to pass these kind of errors from inside my business layer is to store the errors when they occur inside a struct (named either generic for top level errors or a name that relates to the functionality that caused it e.g form-validation) and let it bubble back up to the controller through the return value of whichever service method calls generated them
then i would have the controller inject them into an errorCollection object that lives in the model glue event scope (but would work in the request scope also) and uses a struct to store the returned error struct's.
this then gets passed down into the viewstate where i could then do specific checking on the struct containing the errors related to the section of the page i am working on and if any errors occured they would be displayed
<!--- display page header --->
<!--- display login form --->
bit hard to put it in words in a comment but i hope that makes (some kind-of) sense.
While I have not used Model-Glue, I think I see exactly what you are saying. You are using the ViewState and error collection to really disconnect all the error catching from the view. I think that makes a lot of sense. I am also relatively new to MVC, or to strict MVC, but everything I hear about it makes me think it is a good idea.
as far as i can see putting it into the request scope would work just as well so theres not really anything that ties it into modelglue (the viewstate is just modelglues way of passing information in when it renders its views, in a homegrown app you'd just get it out of the request scope or something.
the reason i came to this conclusion as well is that i'm desperately trying to prevent my service layer from knowing anything about what it is that is accessing it so for each method you call on a service object you either get the expected result or a collection of errors, it's up to whatever is interfacing with the service layer to decide what to do with it.
of course within the service method there is scope to check for errors further down the tier and halt any processing, likewise the controller can check any service call and decide whether it should continue in case the error it encounters is a show-stopper or not.
I just found a nice article on form validation in an OOP context:
It creates a validation object and then passes it into the various objects. These objects then run their own specialized validation rules and populate the passed in error collection objects.
I like this; it removes the requirement to merge multiple error collections together and it removes the need for the various Validate() methods to be able to create their own instances of any error collection objects.
Often, application designers will just ignore this challenge, and accept that validation will have to be duplicated across multiple pieces of code. However, there are some other approaches:
* put the validation in a separate Rules component, that can be re-used by the UI and the other layers -- probably the best solution, but also the hardest to implement.
* put the validation in the business domain objects, or the controller logic, and hope that programmers do not bypass the layer.
* put validation in the UI, and in the database -- this is more common than you would think, because it is the simplest to implement.
Whatever the choice, it is practically impossible to achieve zero-duplication of validation logic. Depending on your particular needs, you will need to choose the best approach.
Ok, so it looks like really going for a DRY (don't repeat yourself) application that has various tiers of validation is generally not achievable. I am not going to let myself get distracted by reused method calls. This should simplify my thinking.
Seeing as we're speaking about validation and errors, if I do a Keyword search I get the following error cfdumped:
Implicit conversion from data type text to nvarchar is not allowed. Use the CONVERT function to run this query.
SELECT e.id, e.name, e.description, e.contraindications, e.alternate_names, e.date_updated, e.date_created FROM el_exercise e WHERE e.is_deleted = 0 AND ( CHARINDEX( (param 1) , e.name ) > 0 OR CHARINDEX( (param 2) , e.description ) > 0 OR CHARINDEX( (param 3) , e.contraindications ) > 0 OR CHARINDEX( (param 4) , e.alternate_names ) > 0 ) ORDER BY e.name ASC, e.id DESC
I assume it's this part that's the problem:
CHARINDEX( (param 2) , e.description ) > 0
This is too weird. This doesn't happen locally, only in production. Same Database structure (I just checked). I just re-synched the code and re-initialized and still getting the error!
Time for some debugging :)
Strange! I fixed the error by moving the CFQueryParam out of the CHARINDEX() method calls:
CHARINDEX( <cfqeueryparam ... />, e.name ) > 0
... and into a declared variable:
DECLARE @keywords VARCHAR( 50 );
SET @keywords = <cfqeueryparam ... />;
CHARINDEX( @keywords, e.name ) > 0
There should be no difference here, at least as far as I can see. I guess something about declaring a length to the VARCHAR variable helped. Of course, I don't need the length, but maybe there is some implicit length when I leave it out.
I still have no idea why the original way would work fine on the local server and NOT on the production environment. I am pretty sure the databases are the same.
Anyway, thanks for catching the error.