When to alter a function vs when to just write a new one...?

Question

/is n00b

Through the gift of knowledge and expertise encoded here, I am doing my best to avoid n00b mistakes as I learn the basics of programming.

I use functions when I (think I) can in PHP, and keep them somewhat sorted in different includes.

The n00b problem I'm running into now is situations where perhaps 4/5th of an existing function is relevant to a new need. Maybe there are a slightly different set of inputs, or an additional calculation or two in the series, or output needs a different format/structure... but the core of the function is still applicable.

Is there a good rule of thumb regarding when one should bolt-on crap to an original function and when one should (literally) copy & paste most of it into a new function and tweak to fit the situation?

On the one hand I feel bad duping code, on the other I feel bad cluttering up an existing function with stuff not always needed...

Answer 1

A good rule of thumb is: never copy & paste.

If 4/5ths of functions A and B are identical, split them up. Have both of them call a new function C that implements the common part.

If you show the code where you're having this problem, we can suggest how to best split it up.

Answer 2

I agree with @Thomas with the proviso that before splitting out the common bits into a new function which your new function and the altered old one invoke you need to do some impact analysis.

Your project manager, test team & customer will not thank you if your goal of pretty code comes at the cost of significant regression testing of areas of the application that had previously been out of scope.

In these cases where the time/money isn't there to do the "right" thing coding wise I will reluctantly copy & paste or write a new function which uses the old and makes up for the extra 1/5th required and surround this with a comment

>>>>>HACKCIDENT ZONE, 
>>>>>NEXT TIME THIS CODE IS CHANGED THE OPPORTUNITY SHOULD BE TAKEN TO CORRECT THIS HORROR
.....
.....
<<<<END OF HACKCIDENT ZONE - move on nothing more to see here

Its not ideal but it makes me feel a bit better about myself...

Answer 3

Why do you find your functions doing 4/5 of the same work? Clearly, you don't have a good abstraction and responsibility segregation.

A function should always do one thing, do it right and do it well. Let's take the example of an Average function that calculates the average from an array of integers. A naive implementation would sum up all the elements and then divide them by the number of elements. This division has a problem though, it knows how to sum an array of integers. This violates the above constraint. A better implementation would split the Sum() function from Average() and have Average() use Sum() to obtain the sum of an array of integers. A chorally to the phrase "a function should only do one thing" is "a function should only know how to do one thing. For everything else it may rely on other abstractions (e.g. functions)."

What can you gain from a clear abstraction and responsibility segregation?

• Better composability of your code, it's easier to write new functions
• No copy&paste coding, spreading potentially buggy code all over your codebase
• Better readability, a reader is not distracted by implementation details for things that are not directly related to what the function should do
• Allows for transparent performance optimizations

Answer 4

Think DRY or DIE -- Don't Repeat Yourself and Duplication is Evil!

Split up your functions. If you copy 4/5 of a function into 5 different functions across 5 different files and uncover a bug in one of them, chances are you'll forget to update at least one of them.

Answer 5

This isnt such a question about what code in the function is the same but what the purpose of the function is, and whether it matches the purpose of the function you're tring to create.

Consider if you've got a function that counts widgets:

function count_widgets($widgets)
{
    // Counting functionality
}

and you need a new function that counts whatsits, but share the functionality. You could create a function that counts widgets or whatsits:

function count_widgets_or_whatsits($thing)
{
   if( $thing instanceof Whatsit )
   {
     // Special whatsit stuff
   }
   // Counting functionality
}

But that would break the contract of the function. In stead, you shoud create a new function, and extract the common functionality:

function count_widgets($widgets)
{
    return count_things($widgets);
}

function count whatsits($whatsits)
{
    // Special whatsit stuff
    return count_things($widgets);
}

function count_things($things)
{
   // Counting functionality
}

This will:

1. Make your code more readable to someone else. (Someone can easily guess that count_whatsits() will count watsits)
2. Provides a solid contract between what is being called and what is happening. If the process of counting whatsits changes, you can simply modify the count_whatsits() functionality without affecting how widgets are counted.
3. Save you hours of maintenance problems when you change something that affects something else.