Refactoring two methods with almost the same content

Question

I have 2 methods which have almost the same content:

public string Method1(int someInt)
{
    if(someBoolean)
        return "test";

    if(someOtherBoolean)
    {
        return "dfjakdsad";
    }
    else
    {
        string myStr = getString(someInt)
    }
}

 public string Method2(myEnum myenum)
 {
    if(someBoolean)
        return "test";

    if(someOtherBoolean)
    {
        return "dfjakdsad";
    }
    else
    {
        string myStr = getString(myenum)
    }
 }

The differences are the method signatures and the single statement in the else, string myStr = getString

Method1 is called from many places so it has to stay in some way. How would I refactor that?

Answer 1

If your enum can cast to an int, and assuming your getString returns the numeric value of the enum and not the text, then simply:

public string Method2(myEnum myenum) 
 { 
    return Method1((int)myenum);
 } 

As an interesting aside - I've seen this situation coined "same whitespace, different values".

Also as an interesting aside - my head-based compiler says that code won't actually compile :-)

I wouldn't worry about refactoring this - the gain is slightly better code readability and possibly, if changes were to occur, decreased complexity on a change - however it's not complicated code so the additional testing overhead if it's used a lot might outweight the gain.

Update: if the enum getString returns the text, ie "Foo", then you cannot refactor this code in the manner I've described.

Answer 2

I would recommend using a generic:

public string Method<T>(T arg)
{
    if(someBoolen)
        return "test";

    if(someOtherBoolean)
    {
        return "dfjakdsad";
    }
    else
    {
        string myStr = getString(arg)
    }
}

This assumes that getString is itself generic or can handle any type of object.

Answer 3

Would casting the enum and calling down to the int version of the function work in your scenario?

public string Method(myEnum myenum)
{
    return Method((int)myenum);
}

public string Method(int someInt) {
    if(someBoolen)
        return "test";

    if(someOtherBoolean)
    {
        return "dfjakdsad";
    }
    else
    {
        string myStr = getString(someInt)
    }
}

Answer 4

You could pass a Func as the second parameter

public string Method(Func<string> myFunc)
{
    if(someBoolen)
        return "test";

    if(someOtherBoolean)
    {
        return "dfjakdsad";
    }
    else
    {
        string myStr = myFunc();
    }
}

Method(myEnum => getString(myEnum));
Method(someInt => getString(someInt));

The generic is the best solution if you are always calling getString as answered by JSBangs.

Answer 5

Is the code in question working, tested code? If so, don't change it without a good reason. I would ask myself two questions:

1) Is making your code a few lines shorter and slightly less redundant really worth the risk of accidentally introducing a bug?

2) Is this the place in the code where my limited time, effort and skill can be best applied?

If this is the worst problem in your code then congratulations, you have awesome code. I would be looking somewhere else for bigger fish to fry.

Answer 6

I would propose the inverse of Adam's answer:

Enums are strongly-typed, whereas int's are not--so you should promote using the enum whenever possible. Therefore I would have the "real" method require an enum as the parameter, and an overload which accepts an int and casts it to the enum.

Note that both methods share the same name, and the enum one is placed first: this means intellisense will "suggest" the enum method first but show the int as an available option:

public string Method1(myEnum myenum)
{
    if(someBoolen)
        return "test";

    if(someOtherBoolean)
    {
        return "dfjakdsad";
    }
    else
    {
        string myStr = getString(myenum)
    }
 }

public string Method1(int someInt)
{
    return Method1((myEnum) someInt);
}

---

Update: As Adam asks, this solution does carry the implication that you always will pass an int which corresponds to a member of your enum. In my experience I this is the common practice (if the enum only maps to a subset of valid ints, then what value does the enum provide?)--however it's worth making sure it fits your use-case.