Re: [Rails-devel] Token.init() and SpecialProperty.init()

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 422-6466

Brett-
again I am disagreeing with you that this code was redundant, it was essential 
to get the game (integration) tests running. The redundant code was the 
initialization of the 

However I agree that static class variables are evil for any kind of testing.
The ids of the tokens should have done from a controller/manager object 
instead of a static class. 

As it is an all or nothing thing here you will either leave all tests working 
again (after re-init) or none, thus I am happy to see

To test this you should re-initialize the test after applying your code 
changes and then change the report output somehow and see if the tests are 
still working.

Unfortunately this will make any change from two weeks ago until your fix 
untestable, so I hope for a soon completion of the refactoring.

Stefan

On Tuesday, July 05, 2011 09:35:43 am brett lentz wrote:
> Stefan -
> 
> Unfortunately, it's simply not possible to leave those .init() methods
> in place. They were part of a key area of the Game class I've been
> reworking. If you take a look at the current state of the Game and
> ComponentManager classes, you should already be able to see
> improvements in the constructors. There is a clear reduction in
> complexity, as well as several static variables and methods have
> already been made unnecessary. This simplification should, in the end,
> make unit testing easier.
> 
> While I agree that breaking the tests is sub-optimal, I disagree with
> you that we should leave redundant code around merely to prevent unit
> tests from breaking. I think we all agree that the preferred solution
> is to remove the static typing.
> 
> I'm hoping to have time over the next few days to continue reworking
> much of the initialization code that caused the over-abundance of
> static declarations. As I'm doing this, I'll take a look at the
> existing tests and try to ensure as many of them stay usable as
> possible.
> 
> ---Brett.
> 
> On Mon, Jul 4, 2011 at 10:26 PM, Stefan Frey <ste...@we...> wrote:
> > Brett:
> > sorry to object: This code is necessary to run the integration tests.
> > It is a workaround to run multiple tests in sequence without starting a
> > new JVM. As Erik mentions this is related to the fact that it uses
> > static class variables.
> > 
> > I have attached again the document (from March 2010) on testing, which
> > include more details on pitfalls to avoid.
> > I will move that to the Wiki as soon as I have more time to do that.
> > 
> > Can I ask you to reverse that change, as I have too many changes in my
> > workspace, which are not ready for a commit?
> > 
> > Thanks in advance,
> > 
> > Stefan
> > 
> > On Tuesday, July 05, 2011 12:04:24 am brett lentz wrote:
> >> Excellent. Then that's what I'll do. :-)
> >> 
> >> ---Brett.
> >> 
> >> On Mon, Jul 4, 2011 at 1:58 PM, Erik Vos <eri...@xs...> wrote:
> >> > No, you're not missing any deep thoughts here. Of course one of these
> >> > initializations is completely redundant.  Can't tell how it became
> >> > this way - I suppose one day I have overlooked it while doing some
> >> > refactoring.
> >> > 
> >> > The bad news that these indexes are static.  These should IMO be moved
> >> > to some instance in the object creation chain.
> >> > 
> >> > Erik.
> >> > 
> >> >> -----Oorspronkelijk bericht-----
> >> >> Van: brett lentz [mailto:bre...@gm...]
> >> >> Verzonden: maandag 4 juli 2011 22:12
> >> >> Aan: Development list for Rails: an 18xx game
> >> >> Onderwerp: [Rails-devel] Token.init() and SpecialProperty.init()
> >> >> 
> >> >> I'm positive I'm missing something here.
> >> >> 
> >> >> In both Token and SpecialProperty, we're defining some static class
> >> > 
> >> > variables,
> >> > 
> >> >> such as:
> >> >> 
> >> >> private static int index = 0;
> >> >> 
> >> >> Then, in the init() method, we're re-defaulting it to the same value
> >> >> we've already declared, such as:
> >> >> 
> >> >>     // initialize the special properties static variables
> >> >>     public static void init() {
> >> >>         tokenMap = new HashMap<String, TokenI>();
> >> >>         index = 0;
> >> >>         log.debug("Init token static variables");
> >> >>     }
> >> >> 
> >> >> 
> >> >> Why is this necessary? Is the defined default value insufficient in
> >> >> some
> >> > 
> >> > way
> >> > 
> >> >> that isn't immediately obvious?
> >> >> 
> >> >> ---Brett.
> >> > 
> >> > ----------------------------------------------------------------------
> >> > --- --- --
> >> > 
> >> >> All of the data generated in your IT infrastructure is seriously
> >> >> valuable. Why? It contains a definitive record of application
> >> >> performance, security threats, fraudulent activity, and more. Splunk
> >> >> takes this data and makes sense of it. IT sense. And common sense.
> >> >> http://p.sf.net/sfu/splunk-d2d-c2
> >> >> _______________________________________________
> >> >> Rails-devel mailing list
> >> >> Rai...@li...
> >> >> https://lists.sourceforge.net/lists/listinfo/rails-devel
> >> > 
> >> > ----------------------------------------------------------------------
> >> > --- ----- All of the data generated in your IT infrastructure is
> >> > seriously valuable. Why? It contains a definitive record of
> >> > application performance, security threats, fraudulent activity, and
> >> > more. Splunk takes this data and makes sense of it. IT sense. And
> >> > common sense. http://p.sf.net/sfu/splunk-d2d-c2
> >> > _______________________________________________
> >> > Rails-devel mailing list
> >> > Rai...@li...
> >> > https://lists.sourceforge.net/lists/listinfo/rails-devel
> >> 
> >> ------------------------------------------------------------------------
> >> --- --- All of the data generated in your IT infrastructure is seriously
> >> valuable. Why? It contains a definitive record of application
> >> performance, security threats, fraudulent activity, and more. Splunk
> >> takes this data and makes sense of it. IT sense. And common sense.
> >> http://p.sf.net/sfu/splunk-d2d-c2
> >> _______________________________________________
> >> Rails-devel mailing list
> >> Rai...@li...
> >> https://lists.sourceforge.net/lists/listinfo/rails-devel
> > 
> > -------------------------------------------------------------------------
> > ----- All of the data generated in your IT infrastructure is seriously
> > valuable. Why? It contains a definitive record of application
> > performance, security threats, fraudulent activity, and more. Splunk
> > takes this data and makes sense of it. IT sense. And common sense.
> > http://p.sf.net/sfu/splunk-d2d-c2
> > _______________________________________________
> > Rails-devel mailing list
> > Rai...@li...
> > https://lists.sourceforge.net/lists/listinfo/rails-devel
> 
> ---------------------------------------------------------------------------
> --- All of the data generated in your IT infrastructure is seriously
> valuable. Why? It contains a definitive record of application performance,
> security threats, fraudulent activity, and more. Splunk takes this data
> and makes sense of it. IT sense. And common sense.
> http://p.sf.net/sfu/splunk-d2d-c2
> _______________________________________________
> Rails-devel mailing list
> Rai...@li...
> https://lists.sourceforge.net/lists/listinfo/rails-devel