In this case - a simple function that takes only one argument and can be handled with a simple
switch statement - you should do that. You gain nothing from breaking this down into separate functions except extra code to maintain. A function should perform one operation clearly and succintly - a simple switch statement such as yours certainly does not break that rule.
By breaking your logic into specific functions for the different colors, you also weaken your design by complicating your program logic in other places: When you change colors, you will have to arbitrarily know (never good) or include logic to determine which function you need to call that point. With your parametrized function it will be much easier to call the function at run-time and dynamically manipulate colors without having to know which particular function you need to call.
Do a "cost/benefit analysis" when designing your functions: Is this function becoming too long and complex? Is trying to do too many things at once? Does it correcly express the idea you want to implement? Will the next developer who deals with this code understand it easily? Be able to modify it easily? Use it in other situtations? Your current function with the
switch meets all these criteria (except for the last one - more on that below.)
However, what does merit some consideration is your first line:
I understand that you need to remove the old color before setting the new one and it is convenient to wrap the call to
removeColour together with setting the new color. But this might be an example of what we call "bundling" - combining several not necessarily related operations into one function.
removeColour belongs in this function - it is an essential element of the
changeColouroperation. So my point here is simply didactic. But it is all too common to find code that is guilty of "bundling" in such manner when that is not the case: Simply bundling a group of unrelated operations together for convenience's sake.
Such a practice leads to difficult to maintain, not robust code: The time will come when you need to execute only one or some of the unrelated operations, and not all of them. When that happens you will have to either duplicate code or change the bundled code and break it out into separate functions - extra work/time/money. That is one (of many) sorts of scenarios when you should be thinking about writing separate functions to handle different operations.
Something else to consider is this line:
Perhaps another example of "bundling": By setting the color of
this within this function, you have restricted its scope and usability: Now the function only works for
this but not for other widgets. You might want to consider just having the function return a string with the name of color.
And have the caller use the result to set this.colour(colorName):
colorName = changeColor(blue);
Then you (or someone else) could also write:
colorName = changeColor(red);
Should you do so, besides having to modify the function to return a
string and dealing with the
removeColor operation, you should probably change the name of the function to
GetColorName(colour) - because you are not actually changing the color in the function, but simply getting the string representation of the desired color. A function name should clearly describe what the function is doing.