6
\$\begingroup\$

After researching for an hour regarding SOLID Principles, I tried to do it myself. Please note that most of these codes were from the ideas of others.

I created a simple program which returns specific value from array, depends on what you choose.

public interface IArrayPerformer
{
    int GetAnswer(int[]arr);
}

public class HighestNumber : IArrayPerformer
{
    public int GetAnswer(int[] arr){ //return highest number }
}

public class LowestNumber : IArrayPerformer
{
    public int GetAnswer(int[] arr) {  //return lowest number }
}

public class MiddleNumber : IArrayPerformer
{
    public int GetAnswer(int[]arr) { // return middle number }
}

public class HighestPrimeNumber : IArrayPerformer
{
    public int GetAnswer(int[] arr) { // return highest prime number }
}

public class LowestPrimeNumber : IArrayPerformer
{
    int GetAnswer(int[] arr) { // return lowest prime number }
}

Then, I created a class and enum which only purpose is to choose what action to be done in arrays..

class ActionChooser
{
    public IArrayPerformer imath(MathAction mathAction)
    {
        IArrayPerformer getMath = null;
        switch(mathAction)
        {
            case MathAction.HighestNumber:
                getMath = new HighestNumber();
                break;
            case MathAction.LowestNumber:
                getMath = new LowestNumber();
                break;
            case MathAction.MiddleNumber:
                getMath = new MiddleNumber();
                break;
            case MathAction.LowestPrimeNumber:
                getMath = new LowestPrimeNumber();
                break;
            case MathAction.HighestPrimeNumber:
                getMath = new HighestPrimeNumber();
                break;

        }

        return getMath;
    }
}

public enum MathAction
{
    LowestNumber = 1,
    HighestNumber = 2,
    MiddleNumber = 3,
    LowestPrimeNumber = 4,
    HighestPrimeNumber = 5
}

Lastly, I created a class for making it all possible..

public class Math
{
    private ActionChooser mathChooser = new ActionChooser();
    public int ReturnAnswer(MathAction mathAcs, int[]arr)
    {
        return mathChooser.imath(mathAcs).GetAnswer(arr);
    }
}

Then, I implemented it like this..

    static void Main()
    {
        int[] arr = { 10, 9, 8, 7, 6, 5, 4, 3, 2, 1 };

        //This is what I see when I am implementing .NET built-in codes
        //which is I only have to create object, use its method,
        //pass a small count of arguments, and see the output without
        //knowing the logic behind the codes. I think this is what they
        //called abstraction. Am I right? It's awesome.
        Math math = new Math();
        Console.WriteLine(math.ReturnAnswer(MathAction.LowestNumber, arr));
        Console.ReadKey();
    }

Now, my question is, am I doing it right or wrong? If i'm wrong (which is I know most probably), which part of this code is wrong. Could you explain why and suggest a better solution.

\$\endgroup\$
6
\$\begingroup\$

am I doing it right or wrong?

You are mostly right. This is a very common pattern but your naming is a little bit different to standard implementations. I should note that in this particular case, this is completely overkill but I'm assuming that this is a learning exercise for you and not something you are planning on using.

public interface IArrayPerformer
{
    int GetAnswer(int[]arr);
}

IArrayPerformer is a bad name because it's very general and it reveals the implementation details. The interface could be made more general by working on an IEnumerable<int> which is the more general interface for a sequence of numbers.

public interface INumberSequenceOperation
{
    int GetAnswer(IEnumerable<int> numbers);
}

Naming this is pretty tough - you'd probably need several attempts to get something good.

Your ActionChooser is normally referred to as a Factory.

public class NumberSequenceOperationFactory
{
    public INumberSequenceOperation CreateOperation(MathAction mathAction)
    {
        // ...
    }
}

I would remove your Math class entirely. It also violates the D in SOLID because it depends on your chooser directly.

static void Main()
{
    var numbers = new[] { 10, 9, 8, 7, 6, 5, 4, 3, 2, 1 };
    var factory = new NumberSequenceOperationFactory();
    var operation = factory.CreateOperation(MathAction.LowestNumber);
    var lowestNumber = operation.GetAnswer(numbers); 
    Console.WriteLine(lowestNumber);
    Console.ReadKey();
}

As another answer has said while I was typing - LINQ gives you most of what you need out of the box and extension methods fit very nicely here.

Update

How does the Math class violate the dependency inversion principle?

Here is the Wikipedia definition of the dependency inversion principle (emphasis is mine):

A. High-level modules should not depend on low-level modules. Both should depend on abstractions.
B. Abstractions should not depend on details. Details should depend on abstractions.

Your Math class is supposed to be an abstraction but it's depending on the detail directly:

private ActionChooser mathChooser = new ActionChooser();

That ActionChooser is a detail but you're tying Math to it! It should be possible to replace that dependency with a different one if you want. The standard way of doing that is with Dependency Injection. e.g. using Constructor Injection:

// Using your names
public class Math
{
    private readonly ActionChooser chooser;

    public Math(ActionChooser chooser) 
    {
        this.chooser = chooser;
    }

    public int ReturnAnswer(MathAction mathAcs, int[]arr)
    {
        return chooser.imath(mathAcs).GetAnswer(arr);
    }
}

In general, you create your entire object graph at your composition root and never create the dependencies again. You can achieve that with "Pure Dependency Injection" (my Main example) or with some kind of Inversion of Control container. There are a lot of these, e.g. StructureMap, Ninject etc.

Learning all of this can be really tough - I'd recommend Mark Seemann's fantastic PluralSight course on Encapsulation and SOLID. If you don't have access to PluralSight and don't want to pay, you can get 3 months free with Dev Essentials. After the 3 months you'll probably decide you want to pay anyway.

Are extension methods bad?

No.

Well, not when they're used properly. There are certainly trade-offs involved in using extension methods but they are extremely effective when used correctly.

Of course, this type of thing is pretty terrible IMO:

// BAD
public static bool IsNullOrWhitespace(this string source)
{
    return string.IsNullOrWhitespace(source);
}

so you can do this:

// Yuck!
string test = null;
test.IsNullOrWhitespace();

Without knowing why you currently believe that they are bad it's difficult for me to make a case for why they're not :)

\$\endgroup\$
  • \$\begingroup\$ Hi RobH, i just wanna ask how Math Class violates the D in SOLID Principle. And extension methods are bad according to some articles I've read. \$\endgroup\$ – Syntax Error Mar 23 '17 at 3:20
  • \$\begingroup\$ @SyntaxError I've added some extra stuff for you \$\endgroup\$ – RobH Mar 23 '17 at 11:58
  • \$\begingroup\$ Well, I cannot agree with the last example. Why is this string extension so terrible? I use this all the time. It's not only much easier to write test.IsNullOrEmpty then test, oh wait, I have to scroll back because I need to check this one for null-or-empty but it's also more natural to read. \$\endgroup\$ – t3chb0t Mar 23 '17 at 15:14
  • \$\begingroup\$ @t3chb0t - you'd have to go back to the beginning of the line to type an if ( anyway. I'll admit - I took a strong dislike to this when extension methods first landed because someone on my team added a bunch like this and it really confused new devs (why doesn't this throw NRE?!) and it meant that they had to dig to double check it was doing what they thought (calling the method on string). It saves very little typing and means an extra using. Perhaps it's time for me to drop this prejudice. \$\endgroup\$ – RobH Mar 23 '17 at 19:27
5
\$\begingroup\$

Yes, that's indeed called abstraction. It's awesome to have a clean and easily understandable code. You did use interface in the proper way. However, you shouldn't reinvent what Microsoft has already implemented in C#.

Your Math class can easily be replaced with either LINQ methods or array extension methods. You don't need a dedicated class just to calculate with the elements of the array and return one particular element.

For example, your LowestNumber function could be written like this:

int[] Numbers = new int[] { 1, 2, 3 };
Console.WriteLine(Numbers.Min()); // Outputs: 1

There are also LINQ functions for Max and Average too, however none for your prime functions. So, you can implement it yourself with extenstion methods this way:

public static int MinPrime(this int[] Numbers)
{
    // ...your code comes here
    return 1; 
}

public static void Main()
{
    int[] Numbers = new int[] { 1, 2, 3 };
    Console.WriteLine(Numbers.MinPrime());
}

LINQ doesn't have a steep learning curve and it's efficient while it keeps our codes clean.

\$\endgroup\$
  • \$\begingroup\$ Hi @bl4y, I didn't mean to implement that kind of algorithm in arrays for real. I just made it an example as for me it was kinda easy. :) Anyway, i don't know if i would accept your extension methods. I have read several articles saying that extension methods are bad. \$\endgroup\$ – Syntax Error Mar 23 '17 at 2:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.