2

Here's a method from my Android app:

public ViewHolderBase buildView(PlayerResult playerResult)
{
    View result = inflater.inflate(
        R.layout.player_result,
        null);
    this.helper = new ViewHelper(result);

    ViewHolderBase holder = createViewHolder();

    TextView playerName = helper.findTextView(R.id.playerName);
    holder.setPlayerNameTextView(playerName); // <-- alleged violation

    TableLayout tableLayout = helper.findTableLayout(R.id.tlPossibleResults);
    populateTableLayout(holder, tableLayout, playerResult);
    holder.setView(result); // <-- another one
    return holder;
}

Android Studio "thinks" that these two lines violate the Law of Demeter:

holder.setPlayerNameTextView(playerName);
holder.setView(result);

I don't get it. Here's the the definition:

The Law of Demeter for functions requires that a method m of an object O may only invoke the methods of the following kinds of objects:

  • O itself
  • m's parameters
  • Any objects created/instantiated within m
  • O's direct component objects
  • A global variable, accessible by O, in the scope of m

So? holder variable is created / instantiated within m.

Fair enough, it's instantiation is delegated to createViewHolder method... Should it make a difference? (side question).

It doesn't for the IDE - if I instantiate holder directly instead, warnings are still shown.

Question:

Is Android Studio wrong? Or is my understanding of Law of Demeter lacking? If the latter is true, how should I refactor this bit to satisfy LoD?

Konrad Morawski
  • 8,307
  • 7
  • 53
  • 91
  • As far as I can see there's nothing wrong in these two lines. – Don Roby Jan 12 '14 at 14:31
  • Can you show pls the createViewHolder method or tell the implementing class of createViewHolder. – Diversity Jan 12 '14 at 14:35
  • @Diversity - `createViewHolder` is abstract in this class (I use abstract method pattern). It's overriden in derived class and the implementation is a one-liner: `return new RangeViewHolder();`. But as I said, even if I replace the `createViewHolder` with a direct, inlined instantiation (`= new RangeViewHolder()`), the warnings persist – Konrad Morawski Jan 12 '14 at 14:48

1 Answers1

1

Neither object is actually instantiated in the "m"; both holder and result are the return value of a function, so technically an object which has been created elsewhere.

Of course you know that "createViewHolder" simply creates an object for someone else's use, and I assume the same is true for "inflater.inflate"; so you can probably disregard those as false positives. But there's no way to programmatically tell an analyst software that, so I'd assume that's why you get the warnings.

One point: since you're delegating the createViewHolder to a child class, you open yourself the code up; thus, I'd still call it a violation of the Law of Demeter. Doesn't mean your code is bad, in my opinion.

Calimar41
  • 222
  • 2
  • 11