开发者

Is this a good way to reuse / share a method?

I encountered this code wherein a method call, for example ClassA.search(a, b, flag) is being used by 3 Controllers. This is a simplified version of the method:

public List<Result> search(Object a, Object b, boolean flag) {
   //do some code logic here, common to the 3 controllers
   //at the middle there is:
   if (flag开发者_JS百科) {
      //code that affects 2 Controllers
   } else {
      //code affects only 1
   }
   //some more common code
   //some more code with the flag if else
}

Is this a good idea because code is reused? Or is there a better way to still be able to make code reuse but not introduce this flag for method caller (client) code customization (like maybe split it to 3 different methods but still be able to declare a common code refactored method)?


First, extract commented lines with functions:

public void search(Object a, Object b, boolean flag)
{
    commonToThree();
    if (flag)
    {
        affectTwoControllers();
    }
    else
    {
        affectsOnlyOne();
    }
    alsoCommon();
}

Now get rid of flag boolean argument, which is a code smell:

public void searchWithTrueFlag(Object a, Object b) {
    commonToThree();
    affectTwoControllers();
    alsoCommon();
}

public void searchWithFalseFlag(Object a, Object b) {
    commonToThree();
    affectsOnlyOne();
    alsoCommon();
}


It is good but not great. One boolean makes sense, but if you start adding more of them you're not going into the right direction.

It's not always possible, but generally yields better code to do:

functionOne:
  sharedCodeOne()
  specificCode
  sharedCodeTwo()

functionTwo:
  sharedCodeOne()
  specificCode
  sharedCodeTwo()

As always, it's hard to make generalized claims: this is obviously not always possible/practical.


It is a relatively simple way to do it. There are alternatives but they are likely to be more complex. (Such as passing a visitor or calling a method of the controller to say what to do for that controller)

This approach is best if you share local variables between the three sections of code and you would have to use fields instead.


I'd take a different approach trying to answer this question in a general way:

The main goal should be clean code. What exactly this is, of course depends on the specific case. But for sure, it's bad to have copy-and-pasted code in several places, since this requires to change several places if it has to be changes - as well, it's bad to try to extract common parts whereever more than one part is using them in all circumstances.

Always imagine someone having to read your code (or yourself having to do this a few weeks from now) and having to understand as fast as possible what's going on there. So, it depends on the special case, if it's better to copy some lines or to extract them in a common method.

A flag is not per se bad, but it's not really something which should be overly used in java methods. Often such a situation can be solved nicely by overloading a method and using one in the other, so that the common case is done in one and the special addition in the other. Alternatively, you can have several sub-methods and compose your method with several calls to them, but this only makes sense, if you don't need to pass too many parameters.

One rule (completely subjective, but based on the lessons-learned from many projects) I can give you: Favor concrete implementations ober generic approaches. They may result in more code and may appear less clever, but they are much easier to understand, extend and debug.

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜