2

I'm making a simple web form to gather customer data and enter it into a database. I have 5 sub-classes: Customer, Bank, Employee, Owner, and TradeReference which inherit from the abstract class, DataEntry. DataEntry has a function public void InsertSelfIntoDataBase(int id);. The id parameter is the primary key from the Customers Table (Bank, Employee, Owner and TradeReference have a many-to-one relationship with Customer), so a Customer doesn't need an id to be inserted (CustomerID is auto Incremented in the database).

Currently, my code is set up so that Bank, Employee, Owner, and TradeReference implements the InsertSelfIntoDataBase function in the parent class, while Customer throws a NotImplementedException, so the code for the Customer class code looks a little like this:

public int InsertSelfIntoDataBase()
{
    int customerID = InsertCustomerAndReturnScalor();
    return customerID;
}


public override void insertSelfIntoDataBase(int id)
{    throw new NotImplementedException("Customer does not use this function");    }

This implementation works, but it bugs me that I have to use a NotImplementedException; Like I can't shake the feeling that my professors form college somehow know and are silently judging me. Is there a better way of doing this?

Robert Columbia
  • 6,313
  • 15
  • 32
  • 40
  • 3
    If its not critical if it is/is not called and return type is `void` then just leave the body of the method empty. – Igor Aug 29 '16 at 15:10
  • Or just comment out trowing exception with a `//` note – Shiham Aug 29 '16 at 15:11
  • 2
    This is a sign that your class modeling isn't quite right. Maybe you could remove that method from the base class and put it on `ISelfDatabaseInsertable` or something. – recursive Aug 29 '16 at 15:12
  • 1
    Why inherit `Customer` from `DataEntry`? – Jeroen van Langen Aug 29 '16 at 15:12
  • 2
    Your college professors are not judging you silently... I hear them shouting "[LISKOV SUBSTITUTION PRINCIPLE](https://en.wikipedia.org/wiki/Liskov_substitution_principle)!" ;) – Good Night Nerd Pride Aug 29 '16 at 15:21
  • @JeroenvanLangen good point. Descendant classes should retain an "IS A" relationship with their ancestors. A `Customer` is not a `DataEntry`, but perhaps a `CustomerEntry` IS a `DataEntry` – Robert Columbia Aug 29 '16 at 15:21

2 Answers2

5

Regardless of the caveats about the class design as Robert Columbia pointed out, I would like give my thoughts about the NotImplementedException.

In the .NET Framework there is anoter well known Exception, which suits this purpose better - the NotSupportedException. It signals that an operation is not supported by the implementation - rather by design and not by the lack of code implementing a feature.

The NotImplementedException is rather an indidicator, that an implementation will and should be done in the future.

Ralf Bönning
  • 14,515
  • 5
  • 49
  • 67
  • Now I know I defiantly need to change it. Throwing a NotSupportedException would be even worse, because it implies that there is no and will never be a function that inserts a Customer, and that isn't the case. – Michael Kaldwid Aug 29 '16 at 15:32
2

This sort of situation can indicate a less than ideal abstract class model. Perhaps you could implement abstract class DataEntry without the insertSelfIntoDataBase(int) method, and then derive a second abstract class such as SelfInsertingDataEntry : DataEntry that defines the abstract method insertSelfIntoDataBase(int) so that concrete classes can inherit from either one depending on whether or not they implement the method.

With this trick, polymorphism related to other methods would be preserved since any concrete instance (whether or not it implemented insertSelfIntoDataBase) could be cast to type DataEntry.

@Recursive also had a good point in a comment, suggesting moving the insertSelfIntoDataBase method into an interface. You could then keep your DataEntry class hierarchy strictly related to Entry type taxonomy and allow some, none, or all descendants to implement or not implement the interface as they wish without requiring them to switch their parent.

Robert Columbia
  • 6,313
  • 15
  • 32
  • 40
  • So do something like subclass DataEntry with OnetoOneDataEntry (requires no id to insert), OneToManyDataEntry (requires 1 ID to insert), and ManyToManyDataEntry (requires 2 ID's to insert). Maybe work on the names though. – Michael Kaldwid Aug 29 '16 at 15:22