CODEWITHSUNDEEP
c#
Dan Brenner
Dan Brenner

Reputation: 888

C# Avoiding Duplicated Logic

Suppose I had a base class A, with subclasses B, C and D. I have a class, MyClass which contains an overload for each of them. I want a counter to increment any time one of these methods is invoked from anywhere other than inside MyClass. This can be accomplished with a few simple helper methods:

public class MyClass
{
   int counter;

   public doSomethingExternal(B b) {counter++; doSomething(b);}
   public doSomethingExternal(C c) {counter++; doSomething(c);}
   public doSomethingExternal(D d) {counter++; doSomething(d);}

   private doSomething(B b) {...}
   private doSomething(C c) {...}
   private doSomething(D d) {...}
}

This really bothers the dev in me. Is there no way better way we can write those helper methods so that we don't have duplicated logic for every subclass? The solution I envision is something like:

// Magically, we know which type we are and call the correct doSomething
public doSomethingExternal(A a) {counter++; doSomething(a);}

I think this kind of generalization could be accomplished with Reflection, but I have heard that reflection is generally slow and can be complicated to understand. I am curious if there is a more conventional pattern I am overlooking that solves this logic-duplication issue.

Upvotes: 3

Views: 139

Answers (4)

InBetween
InBetween

Reputation: 32740

This really bothers the dev in me. Is there no way better way we can write those helper methods so that we don't have duplicated logic for every subclass?

Why? I actually find your code pretty easy to read. I wouldn't really consider "duplicate code", incrementing a simple counter and delegating to a private method. Consider all the alternatives posted? Does any of them make your life any easier?

Of course you could leverage c# 7's pattern matching, but is it really worth it?

public void doSomethingExternal(A a)
{
    counter++;

    switch (a)
    {
        case B b:
            doSomething(b);
            break;
        case C c:
            doSomething(c);
            break;
        case D d:
            doSomething(d);
            break;
        case null:
            throw new ArgumentNullException(nameof(a));
        default:
            throw new InvalidOperationException();
    }
}

All that to simply avoid duplicating counter++?

Its your call, we are probably only seeing a very simplified scenario of what your code really looks like and the extent of your "duplicated" logic.

Upvotes: -1

John Alexiou
John Alexiou

Reputation: 29244

This is clunky and it can be cleaned up with C# 7.0 and pattern matching, but I am not there yet.

public interface A { }
public struct B :A { }
public struct C :A { }
public struct D: A { }

public class MyClass
{
    int counter;
    private void DoSomething(B b) { ... }
    private void DoSomething(C c) { ... }
    private void DoSomething(D d) { ... }

    public void DoSomethingExternal(A arg)
    {
        if (arg is B)
        {
            DoSomething((B)arg);
        }
        else if (arg is C)
        {
            DoSomething((C)arg);
        }
        else if (arg is D)
        {
            DoSomething((D)arg);
        }
        else
        {
            // If `A` is not of `B`, `C` or `D` types return without incrementing counter
            return;
        }
        counter++;
    }
}

Edit 1

if a common interface or class A does not exist, then you have to use object.

public void DoSomethingExternal(object arg)
{
}

Upvotes: -1

Fabio
Fabio

Reputation: 32445

Another approach using dynamic

public class MyClass
{
    int counter;

    public doSomethingExternal(A value) // A is base class
    {  
        counter++; 
        dynamic dynamicValue = value;
        doSomething(dynamicValue); // Correct overload will be used based on actual type
    }

    private doSomething(B b) {...}
    private doSomething(C c) {...}
    private doSomething(D d) {...}
}

Leaving argument of doSomethingExternal to be a base class, will prevent passing other types to the method.

Upvotes: 2

Jonas W
Jonas W

Reputation: 3250

Well one way you can solve this is to make the classes A, B and C implement an interface. Depending on what the doSomething function is doing you can do something like this.

public interface IFoo
{
   void DoSomething();
}

then you can have one public function in MyClass.

public void doSomethingExternal(IFoo foo)
{
   counter++;
   foo.DoSomething();
}

Upvotes: 2

Related Questions