Recfactoring duplication of codes without adding complexity?

Question

I have the following simple methods of very similar code, I like to extract out the common bits, but not if it adds complexity to the method, can someone give me an example or even links on how this can be achieved?

Here are two examples:

public function findRecipeById($id)  
{
   $query = "SELECT * FROM assets WHERE asset_type = 'recipe' AND asset_id = $id";
   ...fetch query data ...
}

public function findArticleById($id) 
{
   $query = "SELECT * FROM assets WHERE asset_type = 'article' AND asset_id = $id";
   ...fetch query data ...
}

As you can see, these two are just some very simple methods to illustrate my problem, they have almost identical query string, and the only option I can think of is to convert them into the following method:

public function findAssetById($assetType, $assetId)
{
   $query = "SELECT * FROM assets WHERE asset_type = '".$assetType."' AND asset_id = $id";
   ...fetch query data ...
}

So the problem I see is that I now have reduced duplication of code and reduced 2 methods into 1, but at the same time I have also created a method with higher complexity than needed by introducing more parameter.

a few key things that are not illustrated here:

1) the query could be much bigger and

2) the complexity of the refactored method could be much higher with many more params added.

My question for the experts out there is if there's a better way to reduce code duplication in this case without introducing complexity? Or is it better to just leave it as 2 simpler methods instead?

Thanks!

Answer 1

Try to make your code more OOP. This is why Data Mappers or Active Record patterns are so popular. Otherwise, your code violates Single Responsibility principle, it tries to make too much.

Your class should either work with articles or recipes, but not both of the same time. This is typical mistake made, cause people think that each table in DB should have single class that represents it, but your objects are not represenation of DB tables, they should be representation of simple objects or processes.

For the same reason, it is wise to break down code into even more classes, one that works with single object, second that works with collection, in your example that would be Article, Recipe, ArticleCollection, RecipeCollection plus maybe interfaces for those 2 groups.

You might want to make a base class, but only if one type might be polymorphically switched with another somewhere in your app. Don't do it just to remove code duplication, instead use composition.