views:

248

answers:

3

Hi I'm reading programming best practices and it is said that when creating a function we should make it do a only single specific task.

I got model functions that retrieves data and it's related data. Example:

$this->Student->StudentAssignments();

Currently this function retrieves the student's assignments plus the question for each assignment and data about the student. I use them all. My dilemma is if I try to make separate functions that retrieves the related data (student and question datas) it's taxing since I'm producing more calls to the DB.

What would you guys suggest?

+1  A: 

I think you're doing fine. But you should reconsider renaming your function to

$this->Student->getStudentAssignmentsWithQuestions

Or whatever you think fit. I think one should try to do as few calls to the database as possible (I assume you're performing a join somewhere in there), instead of fetching each set of elements by specific methods. This can lead to the fact that you'll get more methods (and therefore have to write some more tests), but I think this is the right way to do it.

To defend the design argument: Your method does just one single task; it fetches student's assignments with each assignment's questions.

Yngve Sneen Lindal
A: 

No, if you're strictly concerned about code refactoring you should break down that blob into simpler functions that perform a single task as you said. Yes, you will hit more your database but considering how easy is to work with caching in cakephp, performance should not be an issue. And if it is, then you shouldn't worry about code refactoring at this point.

pcp
A: 

Something to keep in mind when doing this sort of refactoring...

I typically will have a Model->getSomethingAndSomethingElse functions in my models. These functions are public and meant to be called as a substitute for doing complicated (or any) find calls from the Controller.

What I will usually do is then build up a small collection of private functions in the model. In your case I might have something along the lines of...

Student->getStudentAssigmentsWithQuestions

that then calls some private functions i.e.

Student->getStudent which might call Student->joinStudentAssignment which in turn might call Assignment->joinAssignmentQuestion etc.

The double underscore prefixes have been removed since markdown wants to bold things because of them. If you are using php5 the underscores aren't really important anyways as long as you use the "private" or "proteced" keywords.

Basically I use the public method as a container for a group of very specific query building or association building private functions within the models. This allows me to have an api that has complex data returned, but I build the query or the result set (depending on the type of data, relationships involved or query complexity) from small pieces - that can ideally be purposed and used in more than one public function call.

Abba Bryant