CODEWITHSUNDEEP
c#oopsolid-principles
Omu
Omu

Reputation: 71198

is this violating the SOLID principles?

i have something like this in my project, the project it's kinda finished already (it's working) i just want know if it is ok with the SOLID principles 

static public class Tools
{
    static public GetProduct(this id){...}

    static public GetProductCategory(this id){...}

    static public GetUser(this id){...}

    // I also have here methods like IsStringNull ...
    // IsNull IsFalse, lots of stuff, everything static
}

and the usage is like this

var UserThatCreatedCategoryForThisProduct = 
      prodId.GetProduct().CategoryId.GetProductCategory().Creator.GetUser();

i know that is obvious that it's violates the SRP, but this class is static and it contains static methods that are independent one from each other, and it's kind off the same if i would create a static class for each method

Upvotes: 3

Views: 2448

Answers (3)

Ruben Bartelink
Ruben Bartelink

Reputation: 61795

Based on your examples, there's definitely an ISP and a SRP and probably a Law of Demeter (not SOLID but...) violation going on at the very least.

IMNSHO You're far better off reading articles on SOLID (or buying Agile Principles, Patterns, and Practices in C# by Robert C. Martin and Micah Martin which is excellent throughout and one of the most useful books I've read in recent years) than asking for piecemeal advice on the interwebs for this type of thing.

If you want a shortcut (you don't though - the books and PDFs have examples that explain things very well!), these Hanselminutes podcasts with Uncle Bob are very good:

edit: Nabbed the SRP from Jon Limjap and ppiotrowicz

Upvotes: 2

Jon Limjap
Jon Limjap

Reputation: 95432

Far as I can see, there are a lot of SOLID violations right here!

  • Violations of Single Responsibility Principle - First you have data access methods for several classes, second you have helper methods (IsStringNull, IsNull, etc) intertwined with them.
  • Violations of Interface Segragation Principle (as mentioned by Ruben) - If I were only concerned for Products, why do I need exposure to methods that get Users?

I'm sure there are some others, but these are the glaring ones.

UPDATE Now that someone commented on it, I think they're right; the code above looks like it's some form of extension method abuse.

For example I don't believe Data Access should be relegated to extension methods, or worse, a class named "Tools".

It'd probably make more sense to have a base class (on a totally different namespace and/or assembly) that abstracts your data access generalities, then inherit one data access class for each unique domain object (e.g., UserDAO, ProductDAO, etc). Do understand that my assumption here is that by GetProduct or GetUser you actually mean GetFromDatabase.

The rest of the helper methods do belong to extensions so they're fine.

Upvotes: 8

ppiotrowicz
ppiotrowicz

Reputation: 4614

It's violating SOLID principles in many ways.

  • it's not following single responsibility principle, because i't doing at least 3 different things (returning products, returning customers, string operations?)
  • it's violating open closed principle by not being open for extension

Upvotes: 0

Related Questions