I am using CodeIgniter but this question applies in a general sense too.
I have a table of transactions with columns
item_name | type | date | price | document
I want to do the following in two completely independent cases.
1) Get a list of transactions within a certain date range. 2) Get the total price of each transaction.type within a certain date range.
The former can be achieved by simply using a select statement with > datetimestamp
The latter can be achieved by selecting the SUM, and grouping by the type whilst like implementing any required where conditionals e.g with > datetimestamp
Although a simple case, to achieve this I need to have two methods however the bulk of both of these methods (namely the WHERE clauses) are duplicated across both methods.
In terms of speed etc it does not matter but it seems like pointless code reproduction.
A second example is as follows.
I previously had a method get_data($ID) which would get a row from a table based on the ID passed in.
As such in a separate method I would get my 100 items for example.. return an array, loop through them and call get_data for each.
This setup meant that many different methods could get different lists from different sources and then still use the same get_data function and a loop to get the required data.
This minimized code duplication but was incredibly ineffiecient as it meant looping through loads of data items and hundreds of db queries.
In my current setup i just join the data table in each of my methods - code duplication but clear improved efficiency.
A final example is as follows
In codeigniter I can have a function such as the following:
get_thing($ID)
{
$this->load->database();
$this->db->where('ID',$ID);
$this->db->get('table');
}
BUT in alternate situations i might want to only get items in a specific folder.. as such making the function more generic works better.. e.g.
get_thing($array)
{
$this->load->database();
$this->db->where($array);
$this->db->get('table');
}
but then I might want to use this function in two different contexts e.g a user page and an admin page whereby admins can see all items, even unverified ones. My code now becomes:
get_thing($array,$show_unverified = false)
{
$this->load->database();
$this->db->where($array);
if($show_unverified == false)
{
$this->db->where('verified','YES');
}
$this->db->get('table');
}
As you can probably see this can quickly get out of hand and methods can become overly complex, confusing and full of conditionals.
My question is as follows - What are best practices for minimizing code duplication, and how could they be applied to the above situations? I spent hours and hours trying to make my code more efficient yet I'm getting nowhere because I cant workout what I should really be trying to achieve.
Cheers
My idea on code duplication in database access functions is that it is often better to keep it separate.
My rule here is especially that a function should not return different kinds of data depending on the parameter, for example it should not return a single user sometimes and other times an array of users. It may return error codes (false) though.
It is ok though if the function implements different access levels, which are shared across several pages.
This basically always comes back to common sense. You should try to minimize duplicate code and try to reduce complexity within single function. Keep them small and simple.
So basically everytime you try to generalize a function like this you would have to ask if the problem of duplicate code is bigger than the problem of overly complex functions.
In this case i would stop at your second point and next you could create some wrappers for the most common tasks (but be carefull not to make a maze of wrappers)
//you generic function
function get_thing($array)
{
$this->load->database();
$this->db->where($array);
$this->db->get('table');
}
// a nice and friendly wrapper
function get_thing_by_id($id)
{
get_thing(array('id' => $id));
}
// this is just getting silly. don't go crazy with wrappers, only for very often used things.
// and yes the function name is purposely crazy ;)
function get_thing_verified_by_name_and_city_and_some_more($name, $city, $somethingElse)
{
get_thing(array('name' => $name, 'city' => $city, 'somethingelse' => $somethingElse));
}
This answers the first part of your question. Assuming you're using mysql_fetch_assoc or similar. As you're iterating over the result sets you could store count values in a variable in the loop for the total price of each transaction type.
The second part as long as you're not repeating code ad infinitum which would cause you issues down the line when maintaining the code base, it's OK. For your function you could always test the type of variable being passed to the function and set conditional behaviours accordingly.
Have a look at the factory pattern or strategy pattern relating to software design patterns for further insight.