LearningEvaluation usage of setArgs ambiguous

[GoogleCodeExporter]

There is no bug here but I wanted to raise this issue since I (personally) 
can't tell if the behavior was intentional.

In the 'main' function of LearningEvaluation there are 5 levels each which 
starts off with a cmdLineOptions.setArgs("...") call to set the options for the 
level.

This is fine, but because options persist between runs (they're all stored in 
optionsHashMap) all previous options are kept unless they are replaced by a new 
value.

This might be a problem if someone thinks that each run is separate and adds an 
argument that later runs don't use.  Those runs will still have that argument 
set!

For instance, in the current LearningEvaluation.main(), 'level 1' has '-le off' 
set.  However, 'level 2' doesn't mention '-le' at all.  Did the original author 
mean to have default enemies?  Or did he leave it out knowing that '-le off' 
would propagate from 'level 1'?  I don't know, but it seems like such a 
situation would be bad.

What if someone else were shuffling levels around in the function?  They might 
miss such a subtle detail.

Again, not a real bug.  Just something to consider.

Original issue reported on code.google.com by melin...@gmail.com on 28 Oct 2010 at 10:22

[GoogleCodeExporter]

Yes, thanks for spotting this question up -- it also requires some 
clarification. In the beginning it was only the sequential approach elaborated, 
i.e. it is obvious here what's happening:
CmdLineOptions c = new CmdLineOptions(); //all documented defatuls
c.setEnemies("g:10"); // 10 goombas
c.setInvulnerable(true); // defaults, but 10 goombas and Mario is invulnerable 
to creatures.

WOuld you say that c.setArgs("...") should be invariant w.r.t. place it appears 
and c.setEnemies("g:10") should drop 

In this case the cumulative changes are a bit harder to "collect". Now, the 
class does it for you, and by keeping track what's there, one can tweak the 
levels in a desired way. Otherwise in GamePlay it will be necessary to spot 
each additional parameter explicitly as CmdLineOptions class will not store all 
the changes step-by-step. Yet the initial idea of using hash map was also this 
one: to keep track of previous changes.

I came up with this idea: CmdLineOptions.reset() method, that drops all options 
to defaults and is called whenever required. This will keep current behaviour 
and would allow to have invariants of set of options that uniquely define the 
level.

Original comment by Sergey.K...@gmail.com on 29 Oct 2010 at 9:45

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I can definitely see the convenience of being able to build up options, and 
like your solution of adding a reset().  This way if someone was worried about 
it and wanted to set everything explicitly they could call reset() and not have 
to worry.

Thanks.

Original comment by melin...@gmail.com on 29 Oct 2010 at 9:56

[GoogleCodeExporter]

This issue was closed by revision r642.

Original comment by Nikolay....@gmail.com on 10 Nov 2010 at 7:20

• Changed state: Fixed