C# - Entity handler correct use clean code

Question

I have a question about setting up my service. My main concern is if the design is clean and in order. Whether it meets the SOLID principles and does not spill out on me.

I have entities like order and item. These entities will be added. Each entity has a different structure. However, these entities need all the same methods - store in database, download from storage, calculate correctness, delete, etc. separately, the entity should not be extensible. Entities are then further divided into import and export.

This is my idea:

IBaseEntityHandler

public interface IBaseEntityHandler<T> {
 EntityType EntityType { get; set; }
 Task SaveToStorageAsync(string filePath);
 Task LoadFromStorageAsync(string filePath);
 Task CalculateAsync();
 Task SaveToDatanodeAsync();
 .......
}

BaseEntityHandler

public abstract class BaseEntityHandler<T> : IBaseEntityHandler<T> {
 private readonly IDatabase _database;
 private readonly IStorage _storage;
 EntityType EntityType { get; set; }
 Task SaveToStorageAsync(string filePath) {
 _storage.SaveAsync(filePath);
 }
 Task LoadFromStorageAsync(string filePath) {
 _storage.Load(filePath);
 }
 Task SaveToDatabaseAsync() {
 _database.Save();
 }
 Task CalculateAsync() {
 await CalculateAsyncInternal();
 }
 abstract Task CalculateAsyncInternal(); 
}

BaseImportEntityHandler

public abstract class BaseImportEntityHandler<T> : BaseEntityHandler<T> {
 abstract Task SomeSpecial();
}

OrderHandler

public class OrderHandler : BaseImportEntityHandler<Order> {
 public EntityType EntityType { get; set; } = EntityType.Order;
 public async Task CalculateAsyncInternal() {
 }
 public async Task SomeSpecial() {
 }
}

EntityHandlerFactory

public class EntityHandlerFactory {
 public static IBaseEntityHandler<T> CreateEntityHandler<T>(EntityType entityType) {
 switch (entityType) {
 case EntityType.Order:
 return new OrderHandler() as IBaseEntityHandler<T>;
 default:
 throw new NotImplementedException($"Entity type {entityType} not implemented.");
 }
 }
}

My question. Is it okay to use inheritance instead of folding here? Each entity handler needs to have the same methods implemented. If there are special ones - import/export, they just get extended, but the base doesn't change. Thus it doesn't break the idea of inheritance. And the second question is this proposal ok?

Thank you

• Greg Burghardt: "Is it okay to use inheritance instead of folding here?" --- I'm sorry, but I don't understand what you mean by "folding." What does folding mean?
• JonasH: What you are describing sounds kind of like the repository pattern. But your terminology and examples makes it a bit confusing. It is not clear to me if your types are supposed to represent an entity, or a repository. A method like Task CalculateAsync() makes very little sense in either case. Calculate what? where is the result? And if you have a type with a generic parameter, but are not using that parameter anywhere, you might want to reconsider your design.
• Olivier Jacot-Descombes: Usually the question is about inheritance versus composition. I have never heard the term folding in this context.
• GHP: This kind of code can become over complicated quickly. I'm not telling you to change it, but I've seen this kind of code dropped into code bases sporadically where not all entities reuse the same interfaces, and it just becomes a pointless mess. I would say the only rule of thumb for this sort of thing is to ask your own coworkers how they feel about it, and go with what the majority things is cleanest/best.
• Benyamin Limanto: Sorry to jump in @GHP, JonasH, Oliver, I seen many people only discuss in comment section with no answer? Is it because most of you still want to get more information or this question doesn't need answer but only comment? Thank you

Answer 1

Your question is a bit unclear, but I'll give you my answer.

. Is it okay to use inheritance instead of folding here?

"folding" isn't a thing in this context.

question is this proposal ok?

"ok"? i mean if it compiles and works its "ok". Is it a recommended pattern? I would say no.

These days I think it's commonly held that inheritance is flawed and you should use composition instead.

Why do your handlers all need LoadFromDatabase? couldn't you have a handler that loaded from a file, or an API or didn't load at all? What about one that loads from a database but can't use an IDatabase due to the interface being different?

Inheritance restricts, rather than helps you here.

Similarly, do all your entities need to be of EntityType? why? It's not exposed anywhere

..it meets the SOLID principles..

Yeah you have objects, the inheritance is one way.. I assume you are injecting the IDatabase and IStorage so there is some inversion of control, its fine I guess.

[is it] .. correct use clean code [?]

I'm going to assume you mean does it follow the principles in clean code by Robert Martin. As we know from chapter one, the only measure of code is "WTFs per min"

Obviously you only have example code here, But it generated few WTFs from me. It seems obviously what you are doing and I've seen similar code many times before.

1. "folding" ??
2. databases for everything?
3. all void methods? what do these things do?
4. The entity handler factory only works for one type of entity? Is your use of generics correct?

Most of these come from the fact that its all example code with no context. You would get a better answer if you had a real example and a problem.

I have my own measure, which is "Is it like I would program it". The answer is No. I would not use inheritance.

Answer 2

My main concern is if the design is clean and in order. Whether it meets the SOLID principles

These goals imply a binary nature, i.e. that code is either clean or not, and follows SOLID or not. Clean and SOLID are not binary states, they're a spectrum. You're advised to err as much towards the clean/SOLID side of the spectrum as you can, within reason.

For example, the need for SOLID in a small throwaway console tool with limited lifespan is negligible. For a critical production application with an expected lifespan of years, SOLID becomes essential to keep things running smoothly.

I have entities like order and item. These entities will be added. Each entity has a different structure. However, these entities need all the same methods

There's nothing wrong with creating a base interface that is applicable to multiple entity handlers. However, it's unclear to me whether it is necessary to do so. What is the benefit of having this interface?

I'm not implying there's no benefit, but it is important for you to be able to articulate the benefit of anything you build, since anything you add to the codebase adds a non-zero amount of complexity.

The only real SOLID violation I can see here is the factory's switch:

 switch (entityType) {
 case EntityType.Order:
 return new OrderHandler() as IBaseEntityHandler<T>;
 default:
 throw new NotImplementedException($"Entity type {entityType} not implemented.");
 }

This is a textbook OCP violation. The addition of new entity types will continually force you to update this class' implementation, and (not an OCP violation, a contextual DI violation) it will tightly couple your handlers to this factory.

Additionally, there's a weird duplication of the generic type and the corresponding enum value. You're forcing the consumer of your factory to keep these two in sync and if they don't it all breaks.

enter image description here

The easiest solution here is to simply register those handlers in your DI container and not rely on the factory in the first place, instead just getting them from the usual DI container.

services.AddScoped<IBaseEntityHandler<Order>, OrderHandler>();

And then your consumer can just get that dependency:

public class MyConsumer
{
 public readonly IBaseEntityHandler<Order> orderHandler;
 public MyConsumer(IBaseEntityHandler<Order> orderHandler)
 {
 this.orderHandler = orderHandler;
 }
 public void Do()
 {
 var order = new Order();
 orderHandler.Handle(order);
 }
}

This is just a barebones example since I don't know your bigger picture scenario.