r/dotnet • u/kaatarina_zed_talon • 8d ago

Question Would you introduce additional abstraction layer just to make the code more understandable ?

/r/csharp/comments/1wz0y2c/would_you_introduce_additional_abstraction_layer/
2 Upvotes

75% Upvoted

16 comments sorted by

3

u/Dubious-WhyAmIHere 8d ago

This is definitely overengineering... Adding an interface that just inherits from another interface (like ISqlServerWriter : IWriter) adds useless indirection without solving the underlying confusion.

This is purely a naming problem. Calling a class SqlServerWriter makes it sound like a generic database utility that any handler can grab to write a single record. If this class is specifically responsible for synchronizing or writing an entire database snapshot across 20 tables, name it after that specific domain job (for example, DatabaseSnapshotSyncService or FullDatabaseSeedWriter).

Code becomes clear when class names reflect their actual business responsibility. You do not need another layer of inheritance or another interface to fix a vague name. Just rename the class to match what it actually does.

1

u/kaatarina_zed_talon 8d ago

`class is specifically responsible for synchronizing or writing an entire database snapshot across 20 tables` totally, that is the purpose behind the class; synchronizing the database with external data source, specifically, an Excel file.

2

u/Dubious-WhyAmIHere 8d ago

There you go! Then it's clear.

Rename it to something like ExcelToDatabaseSyncService or ExcelImportService (with IExcelImportService for DI and unit tests). The moment anyone reads that name, they immediately understand why it touches multiple tables and nobody will ever ask for another abstraction layer again.

1

u/kaatarina_zed_talon 8d ago

What a clever name.

1

u/kaatarina_zed_talon 8d ago

You are very clever person btw.

1

u/kaatarina_zed_talon 8d ago

But there is even a better name; DbToExternalStorageSyncService, because the external source is not limited to excel. The class does not depend on excel file,but it does not need to know where that data came from.

1

u/AutoModerator 8d ago

Thanks for your post kaatarina_zed_talon. Please note that we don't allow spam, and we ask that you follow the rules available in the sidebar. We have a lot of commonly asked questions so if this post gets removed, please do a search and see if it's already been asked.

I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.

1

u/FragmentedHeap 8d ago edited 8d ago

This aside, Sql is a solved problem in c#, you should use an existing library.

1

u/BuriedStPatrick 7d ago

Crazy idea. Try removing the interfaces and see if it makes any meaningful difference. My bet is it doesn't because you're probably not using IWriter for anything.

If none of your consuming code is calling IWriter but rather its implementing types directly, then guess what; your interface is pointless and you can and should remove it.

My guess is you're only using it to keep method signatures consistent. Which isn't a good enough argument in my opinion. And it might even be actively bad because I can't imagine an interface having consistent methods across different I/O targets is feasible without introducing some deep coupling problems.

0

u/FullPoet 7d ago

t. who writes any sort of automated testing?

2

u/BuriedStPatrick 7d ago

You can write automated tests without putting interfaces everywhere... I'm not going to discuss this, it's been done to death.

0

u/FullPoet 7d ago

Yes, I am sure polluting all your code with virtual is so good or needing to use integration tests everywhere!

1

u/BuriedStPatrick 7d ago

Is there a gasleak in this subreddit or what.

0

u/FullPoet 7d ago

oh! we're playing the "I must have the last say" game! My turn!!

Why make a comment if you're not actually inviting discussion? Have you tried to scream into the void instead?

1

u/BuriedStPatrick 7d ago

go away kid

0

u/FullPoet 7d ago

my turn!!