Re: Single Responsibility Principal

Re: Single Responsibility Principal

Posted 23 January 2013 - 12:13 PM

Well, it is a rather large amount of code there, but I would suggest instead of having and setting the three variables (output message, status, and title) for every possible if/else condition you instead make that another object and simply return one of them (or JSON encode one of them) at the end of the function.

Then, there are a few different ways you could do this. If you know what every possible return value is at the time of programming (IE none of the text is dynamically generated) you could have your functions return a return code stating what happened when validating portions (for example 1 if the user is banned, 2 if suspended, etc.) and use a lookup table to gather the information you need for the submit() function's return value. This could get a little tedious, and potentially confusing if you have to add new messages later on.

For that reason, I would suggest simply splitting out all the functions according to what they do. For example, you check if the account is suspended, banned, etc. Move all of those if statements to a separate function that returns one of the messaging objects I talked about in paragraph 1, or NULL if there is no message to display. If the returned message is NULL it should be enough to signal you to continue with the submission validation; otherwise you should probably stop there and return the JSON information.

This code updated the output information based on each function that passes running. For every function that fails, you get a non-NULL value back and are able to return that right then and there. If there are only a few instances of function to run for the submission process, then you can easily do each one as I showed above, however if you are using a LOT of functions to get through the submission process, you would likely be best coming up with a standard array/hash (associative array) that you pass to each function, place each validation function in an array, and loop through each one calling it with the default argument. That will greatly reduce the amount of code necessary.