Best Practice for Adding an Additional Parameter to a Function

Question

I'm currently working with a very large system and have been asked to add an additional parameter to a method that's called from over 200 different places directly.

The method signature looks something like this:

public static bool SendMessageAndLog(long id, long userId, string message, string cc="", params Attachment[] attachments) 
{ ... }

I need to be able to log the id of the event this message is associated with. I'm kinda stuck between 2 solutions:

1. Creating a new method that does exactly the same thing but takes the event ID as well, stripping the old method and making it call the new method
2. Adding an optional parameter for the event id and going through and using named parameters for the 200 calls which seems like a massive pain

Are there any more other potential solutions to this? What would be the best practice in this case keeping in mind that I can't refactor this too much.

Answer 1

Create a new method with the additional parameter and move all the code from the original method in it, making it use the additional parameter the way you are supposed to. Leave the original method (without the additional parameter) which would just call the new method with some default value of the additional parameter, basically making it a wrapper method. This, of course, works only if there is a default value of the additional parameter. If there is not, but the value differs from call to call, then you have to bite the bullet and just refactor the whole thing, which should not be too hard, because once you break the signature by adding the additional parameter, the compiler will tell you where else you need to fix it.

In your particular case, if you have the message-event id mapping which does not change for the duration of the program execution, you can just create the map as an internal attribute of the class containing the method, and the first line of your method would be to read the map and get the ID of the event based on the message. You then use it the way you want to. That way, you do not need to change the method signature. It should not even be such a performance hit, because I do not expect that the map will have too many elements.

Answer 2

While I agree with the accepted answer, there's also another two approaches you could adopt depending on the requirements.

1. Method Overloading

You can have multiple methods in a c# class that have the same name and perform the same action but take different parameters. Although admittedly not the best approach for your own scenario due to duplication of code. The method that fires will depend on the parameters you provide.

public static bool SendMessageAndLog(long id, long userId, string message, string cc="", 
                                     params Attachment[] attachments) 
{ ... }

// overloaded method
public static bool SendMessageAndLog(long id, long userId, string message,  string cc="", 
                                     int eventId, params Attachment[] attachments) 
{ ... }

2. Nullable parameters / Assign a default parameter value

This approach allows you to keep the one method, however all the parameters that you assign a default value to must be the right-most parameters in the method. This means if you need to add an additional parameter later on and can't re-write the code calling the method, then the new parameter must also have a default value assigned (Which can be null if you wish.).

You'd also need to remove params from the parameters in your case so again not ideal! Although I'm not a fan of params when you can just pass in a named list or array in its place anyway.

public static bool SendMessageAndLog(long id, long userId, string message, string cc="", 
                                     Attachment[] attachments, int? eventId = null) 
{ ... }

//You can now call this method in two ways...

//1)eventId param not provided so it will default to null inside method block
SendMessageAndLog(id, userId, message, attachments);

//2)eventId value provided so it will be whatever value you provide.
SendMessageAndLog(id, userid, message, attachments, eventId);