Where should I put "ugliness"?

Question

Scenario: I have a function that I need to tweak in some way (example; make it work slightly different in different places). For some reason I end up having to add something ugly to the code, either in the function or at existing call sites. Assume that the sum total "ugly" is the same in both cases.

The question is which choice should I pick and why?

Should I encapsulate it so I don't need to look at it or should I extract it so that it doesn't add semantic trash to the function?

What would effect your choice? What about if:

• The function will "never" be called except from the current locations.
• New calls to the function won't need the "ugliness".
• The function is really clean and generic right now
• The function is already a hack job.
• you wrote the function
• you didn't wright the function
• etc.

Answer 1

Put the ugly in the function, hands down. If this is in C++ be sure to have the implementation in the .cpp file. Perhaps you may consider writing two functions to abstract the ugly from the main function body.

Procedural/OOP programming exists to take the "ugly" out of the interface (among other things). It is important to realize that the more code that is required to call the function the less useful it becomes. Also remember to document the code clearly and note that there is ugly code in there and why.

Also, if this function is big enough you may consider putting it in its own .cs file, or .cpp/.h pair (depending on which language you are using).

Answer 2

If the ugliness isn't needed by new callers, then I'd encapsulate the ugliness in a new function/helper/whatever and call it from the old callers so it isn't polluting the existing function for new callers, and isn't duplicated across old callers.

If it will never be needed for new callers, then I'd add it to the function (if it makes sense in context of the function) since it is essentially a legacy component anyway as long as you can appropriately test the functionality.

Bottom line, I'd try to minimize the ugliness both in having no duplication across multiple callers and minimizing the exposure to new code. I'd encapsulate it to the point where could unit test it.

Who wrote it wouldn't affect my decision.

Answer 3

I would put it in the function declaration personally - I can't grasp how putting it in the calls to the function would be "less ugly", since you'd likely(at least, you implied) that you would have to use the "ugly" more than once.

Answer 4

I'd encapsulate the ugly in a wrapper function and change all the functions needing the ugly to use the wrapper.

//old call
call_some_function_with_ugly(params, case)

// new call
call_some_function(params)

void call_some_function(params){ }

void call_some_function_with_ugly(params, case){
    // do ugly depending on case
    switch(case) { ... }

    call_some_function(params);
}

Answer 5

Absolutely put the ugliness in the function. One of the worst things you can do is spread the ugliness throughout the rest of the code, where it will be isolated in many separate instances. These will be impossible to track down once the ugliness changes in some unforeseeable way (which ugliness happens to often do).

Of course, the best option is to not have any ugliness in the first place... what particular example are you thinking of right now?