Re: Feedback on ForStatement

I tried to make minimal changes when adding index variable to ForStatement.  It seemed that getValueVariable() and getIndexVariable() was the cleanest extension of getVariable() (now deprecated).  One option versus adding a constructor would be to add a "forS" factory overload to GeneralUtils that supports classical "for (int i = 0; i < n; ++i) ..."
 without making reference to the dummy variable.

A bolder approach would be to separate "for (x in y)" from "for (int i = 0; i < n; ++i)" as distinct statement classes.  Maybe they extend from ForStatement to keep visitors happy.


When getIndexVariable() and getValueVariable() were introduced, I designed them so you could use them equally.  That is, they return null when no index or value variable was given.  To that end, I would not want to see hasValueVariable() and hasIndexVariable() introduced.  You should be able to do any "if (hasValueVariable()) f(getValueVaruable())" with "Optional.ofNullable(getValueVaruable()).ifPresent(v -> f(v))" or of course "var v = getValueVariable(); if (v != null) f(v);".

In summary, I'd favor a light tough for a refactoring; add as little as possible to the statement class.  Unless you are willing to separate the abstractions of collection and counting loops.

Eric M.