Cfrgtkky

I found an inheritance tree in our (rather large) code base that goes something like this:

public class NamedEntity

{

    public int Id { get; set; }

    public string Name { get; set; }

}



public class OrderDateInfo : NamedEntity { }

From what I could gather, this is primarily used to bind stuff on front-end.

For me, this makes sense as it gives a concrete name to the class, instead of relying on the generic NamedEntity. On the other hand, there is a number of such classes that simply have no additional properties.

Are there any downsides to this approach?

For me, this makes sense as it gives a concrete name to the class, instead of relying on the generic NamedEntity. On the other hand, there is a number of such classes that simply have no additional properties.

Are there any downsides to this approach?

The approach isn't bad, but there are better solutions available. In short, an interface would be a much better solution for this. The main reason why interfaces and inheritance are different here is because you can only inherit from one class, but you can implement many interfaces.

For example, consider that you have named entities, and audited entities. You have several entities:

One is not an audited entity nor a named entity. That's simple:

public class One 

{ }

Two is a named entity but not an audited entity. That's essentially what you have now:

public class NamedEntity

{

    public int Id { get; set; }

    public string Name { get; set; }

}



public class Two : NamedEntity 

{ }

Three is both a named and audited entry. This is where you run into a problem. You can create an AuditedEntity base class, but you can't make Three inherit both AuditedEntity and NamedEntity:

public class AuditedEntity

{

    public DateTime CreatedOn { get; set; }

    public DateTime UpdatedOn { get; set; }

}



public class Three : NamedEntity, AuditedEntity  // <-- Compiler error!

{ }

However, you might think of a workaround by having AuditedEntity inherit from NamedEntity. This is a clever hack to ensure that every class only needs to inherit (directly) from one other class.

public class AuditedEntity : NamedEntity

{

    public DateTime CreatedOn { get; set; }

    public DateTime UpdatedOn { get; set; }

}



public class Three : AuditedEntity

{ }

This still works. But what you've done here is stated that every audited entity is inherently also a named entity. Which brings me to my last example. Four is an audited entity but not a named entity. But you can't let Four inherit from AuditedEntity as you would then also be making it a NamedEntity due to the inheritance between AuditedEntityandNamedEntity`.

Using inheritance, there is no way to make both Three and Four work unless you start duplicating classes (which opens up a whole new set of problems).

Using interfaces, this can easily be achieved:

public interface INamedEntity

{

    int Id { get; set; }

    string Name { get; set; }

}



public interface IAuditedEntity

{

    DateTime CreatedOn { get; set; }

    DateTime UpdatedOn { get; set; }

}



public class One 

{ }



public class Two : INamedEntity 

{

    public int Id { get; set; }

    public string Name { get; set; }

}



public class Three : INamedEntity, IAuditedEntity

{

    public int Id { get; set; }

    public string Name { get; set; }

    DateTime CreatedOn { get; set; }

    DateTime UpdatedOn { get; set; }

}



public class Four : IAuditedEntity

{

    DateTime CreatedOn { get; set; }

    DateTime UpdatedOn { get; set; }

}

The only minor drawback here is that you still have to implement the interface. But you get all the benefits from having a common reusable type, without any of the drawbacks that emerge when you need variations on multiple common types for a given entity.

But your polymorphism remains intact:

var one = new One();

var two = new Two();

var three = new Three();

var four = new Four();



public void HandleNamedEntity(INamedEntity namedEntity) {}

public void HandleAuditedEntity(IAuditedEntity auditedEntity) {}



HandleNamedEntity(one);    //Error - not a named entity

HandleNamedEntity(two);

HandleNamedEntity(three);  

HandleNamedEntity(four);   //Error - not a named entity



HandleAuditedEntity(one);    //Error - not an audited entity

HandleAuditedEntity(two);    //Error - not an audited entity

HandleAuditedEntity(three);  

HandleAuditedEntity(four);

On the other hand, there is a number of such classes that simply have no additional properties.

This is a variation on the marker interface pattern, where you implement an empty interface purely to be able to use the interface type to check if a given class is "marked" with this interface.

You're using inherited classes instead of implemented interfaces, but the goal is the same, so I'm going to refer to it as a "marked class".

At face value, there's nothing wrong with marker interfaces/classes. They are syntactically and technically valid, and there are no inherent drawbacks to using them provided that the marker is universally true (at compile time) and not conditional.

This is exactly how you should differentiate between different exceptions, even when those exceptions do not actually have any additional properties/methods compared to the base method.

So there's nothing inherently wrong with doing so, but I would advise using this cautiously, making sure that you're not just trying to cover up an existing architectural mistake with badly designed polymorphism.

This is something that I use to prevent polymorphism from being used.

Say you have 15 different classes that have NamedEntity as a base class somewhere in their inheritance chain and you are writing a new method that is only applicable to OrderDateInfo.

You "could" just write the signature as

void MyMethodThatShouldOnlyTakeOrderDateInfos(NamedEntity foo)

And hope and pray no one abuses the type system to shove a FooBazNamedEntity in there.

Or you "could" just write void MyMethod(OrderDateInfo foo). Now that is enforced by the compiler. Simple, elegant and doesn't rely on people not making mistakes.

Also, as @candied_orange pointed out, exceptions are a great case of this. Very rarely (and I mean very, very, very rarely) do you ever want to catch everything with catch (Exception e). More likely you want to catch a SqlException or a FileNotFoundException or a custom exception for your application. Those classes often times don't provide any more data or functionality than the base Exception class, but they allow you to differentiate what they represent without having to inspect them and check a type field or search for specific text.

Overall, it's a trick to get the type system to allow you to use a narrower set of types than you could if you used a base class. I mean, you could define all your variables and arguments as having the type Object, but that would just make your job harder, wouldn't it?

This is my favorite use of inheritance. I use it mostly for exceptions that could use better, more specific, names

The usual issue of inheritance leading to long chains and causing the yo-yo problem doesn't apply here since there is nothing to motivate you to chain.

IMHO the class design is wrong. It should be.

public class EntityName

{

    public int Id { get; set; }

    public string Text { get; set; }

}



public class OrderDateInfo

{

    public EntityName Name { get; set; }

}

OrderDateInfo HAS A Name is a more natural relation and creates two easy to understand classes that wouldn't have provoked the original question.

Any method that accepted NamedEntity as a parameter should only have been interested in the Id and Name properties, so any such methods should be changed to accept EntityName instead.

The only technical reason I'd accept for the original design is for property binding, which the OP mentioned. A crap framework wouldn't be able to cope with the extra property and bind to object.Name.Id. But if your binding framework can't cope with that then you have some more tech debt to add to the list.

I'd go along with @Flater's answer, but with lots of interfaces containing properties you end up writing a lot of boilerplate code, even with C#'s lovely automatic properties. Imagine doing it in Java!

Classes expose behavior, and Data Structures expose data.

I see the class keywords, but I don't see any behavior. This means that I would start viewing this class as a data structure. In this vein, I'm going rephrase your question as

Why have a common top level data structure?

So you can use the top level data type. This permits leveraging the type system to ensure a policy across large sets of different data structures by ensuring the properties are all there.

Why have a data structure that includes the top level one, but adds nothing?

So you can use the lower level data type. This permits putting hints into the typing system to express the variable's purpose

Top level data structure - Named

   property: name;



Bottom level data structure - Person

In the hierarchy above, we find it convenient to specify that a Person is named, so people can obtain and alter their name. While it might be reasonable to add extra properties to the Person data structure the problem that we are solving doesn't require a perfect modeling of the Person, and so we neglected to add common properties like age, etc.