tags:

views:

114

answers:

5

Suppose I have two functions which look like this:

public static void myFunction1(int a, int b, int c, string d)
{
    //dostuff
    someoneelsesfunction(c,d);
    //dostuff2
}

public static void myFunction2(int a, int b, int c, Stream d)
{
    //dostuff
    someoneelsesfunction(c,d);
    //dostuff2
}

What would be a good way to avoid repeated dostuff?

Ideas I've thought of, but don't like:

  1. I could make d an object and cast at runtype based on type, but this strikes me as not being ideal; it removes a type check which was previously happening at compile time.
  2. I could also write a private helper class that takes an object and write both signatures as public functions.
  3. I could replace dostuff and dostuff2 with delegates or function calls or something.
+6  A: 

Just refactor "doStuff" and "doStuff2" into separate methods.

They're obviously doing "stuff" that's separate from the "someoneelsesfunction" method, so they should be separate methods anyways. Each method should have one task, ideally.

This is even a supported refactoring in Visual Studio - just highlight the code in "dostuff", and right click, and choose Refactor->Extract Method.

Reed Copsey
+1 for "they should be separate methods anyways"
Kane
Also +1 for "Refactor->Extract Method" suggestion.
Mark Byers
In the real code, I don't feel that `dostuff`, `dostuff2`, and `someonelsesfuction` really belong in separate methods. In fact, I think the code would be much less readable if I did so.
Brian
A: 

I'd start by extracting methods DoStuff and DoStuff2 and calling them from both functions.

You could then start to get funky by wrapping the block in another method and passing the someoneelsefunction to use to that method, but thats a step too far imho. you want things to be easy to read and change as well as not containing repetition.

Sam Holder
+3  A: 

I might do something like this:

public static void myFunction1(int a, int b, int c, string d)
{
    //dostuff
    someoneelsesfunction(c, d);
    //dostuff2
}

public static void myFunction2(int a, int b, int c, Stream d)
{
    string str = d.ReadEntireString(); // no such method, but basically
        // whatever you need to do to read the string out of the stream
    myFunction1(a, b, c, str);      
}

... and change the name of both functions to MyFunction (to take advantage of overloading, which is the same thing someoneelsesfunction() is doing.

NOTE: this solution would be impractical if the string contained in the Stream is ginormous. If so, you might want to do this the other way around: read the string d into a stream and call the override with the stream parameter.

MusiGenesis
Ack, I feel silly. This isn't actually what the real code does, but the almost the same principle applies.
Brian
By the way, another option is to wrap a string in a `MemoryStream`.
Brian
@Brian: yeah, that's what I meant with my last edit (reading the string into the stream).
MusiGenesis
Also, no need to feel silly. I actually think my answer here would not be workable in a lot of cases. Sometimes you can't really convert one object into another type like this very easily, which is usually why the overloads are there in the first place.
MusiGenesis
+1  A: 

Eventually you can create

public static void myFunction3(int a, int b, int c, Stream d, string e)

which will be called from function1 and function2 with d or e set to null. Then function3 would distinguish between calls to someoneelsesfunction(c,d). It is a possible solution, but for desperates :)

twk
+1 to counteract the inevitable down-votes. IMHO, this isn't functionally any worse than overloading.
MusiGenesis
thanks for understanding the risk ;-)
twk
There were no downvotes :P
Brian
+3  A: 

Use Action to customize what a function does in some specific place.

public void myF1(int a, int b, int c, int d)
{
     internalF(a, b, { someelse1(c, d); }); 
}
public void myF2 (int a, int b, int c, Stream d)
{
     internalF(a, b, { someelse2(c, d); });
}
private void internalF (int a, int b, Action action)
{
    // dostuff
    action();
    // dostuff2
}
Pasi Savolainen
cool :) I like it
twk