Is this a bad practice in overloading a method?

Question

I had a method like this that its consumers are calling it:

static public void DisplayOrderComments(param1, param2, param3, param4)

Now I added an overload for it like this:

static public void DisplayOrderComments(param1, param2, param3, param4, param5)
{
    DisplayOrderComments(param1, param2, param3, param4);
    param5.Foo();
}

Is it a bad practice? Are there better ways of doing it?

Answer 1

This is absolutely fine - it keeps code DRY and avoids unnecessary duplication.

Not only is it not a bad practice, it is a good practice.

---

If you are using C# 4.0 and above (VS 2010+), you can use an optional argument for your param5 instead of overloading, as Mikey Mouse mentions in this answer.

Answer 2

Nice question.

I would say not, it is normal overloading. but I will change it as such (always implement in the one with most parameters):

static public void DisplayOrderComments(param1, param2, param3, param4)
{
   DisplayOrderComments(param1, param2, param3, param4, null);
}

static public void DisplayOrderComments(param1, param2, param3, param4, param5)
{
  ... // do the work
  if(param5!=null)
     param5.Foo();
}

Answer 3

Yeah, I woudln't say it's bad but if you're using C# 4.0 I'd recommend making the last parameter optional.

You can read all about em here http://msdn.microsoft.com/en-us/library/dd264739.aspx

Answer 4

Not at all. This is perfectly acceptable but there are two problems.

1. param5 might be passed in as Null - you might want to write some code to check for that condition and do the appropriate thing.
2. This will prevent you from using optional parameters later. But you might not care about that.

By the way codereview.stackexchange.com might be a better place for this sort of question.