OOP Design: Abstract class design vs Regular Inheritance

Question

For fun, I'm designing a simple system to store records (vinyl records) data and some other general items related to records (sleeves, record cleaners, etc).

Since 90% of the data is gonna be vinyl records and the other 10% other items, I thought about separating them into two classes: Records and Items. Bear in mind that although different at the end both are products with certain things in common like price, quantity, etc.

I have doubts about whether I should create an abstract class, say Item, and two classes extending Item: Records and NonMusicalProducts. Something like this:

abstract class Item {

    static function getItemPrice($itemId, $type='record'){
        $result = 0;
        // query record table if type is record, otherwise query nonmusicalproduct table
        return $result;
    }

    abstract function getItemDetails();
}

Whereas Record:

class Record extends Item {
    static function getItemDetails($id) {
        // query DB and return record details
    }
}

and NMProduct

class NMProduct extends Item {
    static function getItemDetails($id) {
        // query DB and return NMProduct details
    }
}

Would it be ok to define both methods in NMProduct and Record as static? I'm not always gonna access that method from an object.

The other option is to have only an Item class and a Record class that would inherit from item but I have come to a point where it doesn't seem right, especially when trying to get the details:

class Item {
   function getItemDetails($id, $type){
        if ($type == 'record') {
            // query record table using id
        }
        if ($type == 'nmproduct'){
            // query nmproduct table
        }
}

It feels wrong because I think that it would be more appropriated to go to the record class to get record details since the columns would be different to those columns in nmproduct. Doing it in one parent class feels that defeat the purpose of OO.

Or the last option that I can think of, would be one single class Item:

class Item {

     function getItemPrice($id, $type) {
         // if type is record then query the record table
         // if type is nmproduct then query the nmproduct table
     }

     function getItemDetails($id, $type) {
         // if type is record then query the record table
         // if type is nmproduct then query the nmproduct table
     }
}

Again, this last option doesn't feel right since too many non-related things will be condensed in one single class. The record attributes (ie artist_id, number_of_discs, etc) are different than nmproduct.

What would be the best approach to this issue?

Answer 1

Frankly, your question seems off from OOP, and specifically Polymorphism.

Also, it seems not right from entity and data access design standpoint.

From polymorphic standpoint: obviously, you should define a base class with abstract members. Both concrete classes should derive from the abstract base class and should override the abstract members. No static functions.

Before you do that, you need to decide whether this at all makes sense. Are both Record and NMProduct the same thing? I'd say yes actually, because both of them are something that you could sell in a store. Is Item a good common name? maybe not. But regardless of what I think, you need to make the final call on this question.

If they turn out not to be the same thing conceptually, maybe even inheritance does not make much sense. Then keep them separate and duplicate the method names. Point is - inheritance should not be used just for reuse. Inheritance should be used if 2 or more things (concrete classes) are different kinds of the same conceptual thing (base class).

From entity and data access standpoint: entities should not query databases. Repositories should query databases and provide instances of entities. Entity should allow you to work with it, regardless of how it's stored, retrieved, persisted, serialized, and so on. Repositories should encapsulate the "HOW" part.

Usually, when you have an inheritance of the persisted objects (e.g. entities, and ideally aggregate roots - but that is outside of the scope of this discussion), you will also have a similar inheritance of the repositories. So, you will have an ItemRepository, RecordRepository deriving from ItemRepository, and NMProductRepository deriving also from ItemRepository. RecordRepository and NMProductRepository will only retrieve Records and NMProducts respectively. ItemRepository will retrieve both, and will return all of them together as Items. This is legitimate and possible because both the Record and the NMProduct are Item.

I hope this is clear even without code samples.

Answer 2

You have to see the Strategy Design Pattern for this , because your scenario exactly fit in it.

Answer 3

Personally, what you want to have at the end of the day is a list of items, regardless of type. So you definitely want to have a common base class - this will allow you to handle all of the objects in at least a basic fashion.

Consider Generics for this problem as well, as in...

public class Item<T extends IHaveDetails> {
    T details;

    public Price getPrice() {
        return details.getPrice();
    }

    public Details getDetails() {
        return details.getDetails();
    }
}

Another approach is composition of a different type - using a container. Essentially, you create the item class so that it only has one data member - a Map of details by name.

You then populate the map with whatever Details you want. If the details are stored in a related table, with one detail per row, you can then store items of any type, containing any number of details, using one simple class. Consider...

public class Item { private Map details = new HashMap<>();

 public Detail getDetail(String name) {
     return details.get(name);
 }

 public void addDetail(Detail detail) {
     return details.put(detail.getName(), detail);
 }

}

Using the above class would look like...

Item record = new Record();

record.addDetail(new NumericDetail("price", 10.00));
record.getDetail("price");

Good Luck!

Answer 4

I'd create 2 classes with different getDetails and getPrice implementation:

interface Detailable {
  public function getDetails($id);
  public function getPrice($id);
}

class Record implements Detailable{
  public function getDetails($id) {
    // custom Record Details
  }

  public function getPrice($id) {
    // custom Record price
  }
}


class NMProduct implements Detailable{
  public function getDetails($id) {
    // custom NM Product Details
  }

  public function getPrice($id) {
    // custom NMProduct price
  }
}

And next pass an NMProduct or Record instance into the constructor to Item:

class Item {

  protected $_type;

  constructor(Detailable $type) {
    $this->_type = $type;
  }

  function getItemPrice($id) {
    return $this->_type->getPrice($id);
  }

  function getItemDetails($id) {
    return $this->_type->getDetails($id);
  }
}

So, real db queries are delegated from Item to concrete implementation, which is different for NMProduct and Record.

Answer 5

There is also another maybe even more preferable option to achieve polymorphism: Composition.

But of you provided options I would prefer the first. But make your base classes as general as possible (since it's your base class and you don't want to narrow down usage because your base classes are too specific):

First example:

abstract class Item
{
  int Id;
  abstract Decimal GetPrice();
  abstract IItemDetail GetDetail();
}

Since in your first example fetching data from the database is type specific, it would be better to move type specific operations to the type itself (hence each derived type must implement its specific behavior. This eliminates type checks). And because of this, making those methods static in the base class doesn't make much sense (in contrast to where fetching data always works based on a common type). But its a design decision.

Your second example introduces those nasty type checks mentioned before. They make your code difficult to extend and hard to understand (e.g. readability). This is because, as said before, you moved type specific operations (that can't be generalized e.g. templates) into a type (can be a base type) that doesn't have all information required to execute an operation. This type doesn't know anything about the object. Neither type nor state. Before operations can take place, this type needs to query the target for all information: What type is the object? What state is it in? That's why we apply the Tell-Don't-Ask principle and inheritance to get rid of this.

The third example is more like a static helper class. Methods could be static (and when supported generic or templated) as this helper type is intended operate on any type (or a common base type). But again, if operations are type dependent (like in your example) than this would introduce type switching and maybe state checking which is a big step away from writing extensible/ maintainable code.

Your first option of those you provided is the cleanest. It allows polymorphism. And it doesn't violate Law of Demeter or Open-Close. Adding a new type, for example "Movie", is done by implementing a new "Item", that's very important regarding extensibility.

But there are more possibilities to achieve what you are trying to do. I said Composition before. But what about encapsulation? We could use Encapsulation to extract the varying part. We could have a single "Item" type, which is used for all sort of items and extract the type specific behavior (the only behavior where our subtype implementations will differ, e.g. the DB access in your example) into a new type:

class Item
{
  // ctor
  Item(IItemPlayer typeSpecificPlayer)
  {
    this.TypeSpecificPlayer = typeSpecificPlayer;
  }

  public void Play()
  {
    this.TypeSpecificPlayer.Play(this);
  }

  public int Id;
  private IItemPlayer TypeSpecificPlayer;
}

interface IItemPlayer 
{
  void Play(Item itemToPlay);
}

class RecordIItemPlayer implements IItemPlayer 
{
  void Play(Item item)  { print("Put the needle to the groove"); }
}

class MovieIItemPlayer implements IItemPlayer
{
  void Play(Item item)  { print("Play the video"); }
}

The constructor forces injection of the varying components for the Composition of the type "Item". This example made use of Composition and Encapsulation. You would use it like:

var record = new Item(new RecordPlayer());
var movie = new Item(new MoviePlayer());

recordItem.TypeSpecificPlayer.Play();
movieItem.TypeSpecificPlayer.Play();

Without the use of Composition the "IItemPlayer" becomes an associated external type:

interface IItemPlayer
{
  void Play(Item item);
}

class RecordIItemPlayer implements IItemPlayer 
{
  void Play(Item item)  { print("Put the needle to the groove"); }
}

class MovieIItemPlayer implements IItemPlayer
{
  void Play(Item item)  { print("Play the video"); }
}

class Item
{    
  int Id;
  Decimal Price;
  IItemDetail Details;
}

and use it like:

var record = new Item();
var recordItemPlayer = new RecordItemPlayer();

var movie = new Item();
var movieItemPlayer = new MovieItemPlayer();

recordItemPlayer.Play(record);
movieItemPlayer.Play(movie);

If need additional variations, e.g. ways to play certain type of media, we would just add a new "IItemPlayer" implementation.