[messages] [Developers] Re: [Developers] Vassal code maintainability

fil512 ken.stevens at sympatico.ca
Fri Jul 30 09:32:12 MST 2010


"uckelman" wrote:
> Thus spake fil512:
> > Hey guys, I'm new to the Vassal coding world and have a bunch of
> typical
> > newbie questions.
> > 
> > 1. What's with all the nio/zip source code?  It looks like it has
> > nothing to do with Vassal.  Shouldn't that be a jar dependency
> instead
> > of source code?
> 
> No, I wrote almost all of that becuase there's no backport of Java 7's
> ZipFS
> for Java 6. But presently we're not using any of in fact, I've had so
> much
> difficulty with it that I'm in the middle of backing out all of it.
> 


Good to see it's being yanked out.  It looks like a bunch of code that
isn't concerned with Vassal stuff--looks like something that there
should be a jar for. 


"uckelman" wrote:
> 
> > 2. Unit tests anyone?  I see a huge whack of unit tests under nio,
> but I
> > assume those aren't authored by Vassal coders...  Without unit test
> > coverage, changing the Vassal engine code is like flying a trapeze
> > without a net.  How will you know if your change breaks any existing
> > modules?
> 
> No, I wrote almost all of those tests. I completely agree with you
> about
> making modifications when we have very little test coverage. Most of
> the 
> code was written before anyone was thinking about writing tests, and a
> good chunk of the code is just untestable.
> 


I have yet to see a piece of code I can't test.  It's true that with all
the UI stuff there it's harder to test than it should be, but I found
that with a few mocks I was able to test everything that needed to be
tested.  Not sure if you've looked at the Vassal module analyzer I
wrote, but it's written completely as JUnit tests and uses mocks to deal
with obstacles to unit testing.


"uckelman" wrote:
> 
> > 4. Visitor pattern.  A bunch of the confusion I've seen about how
> traits
> > are resolved stems from mixing visting algorithms with the
> structures
> > that those visiting algorithms operate on.  I.e. I don't think
> traits
> > should be responsible for doing things to other traits.  Traits
> should
> > just change state and send and receive events.  Another layer on top
> of
> > traits should then be responsible for the order traits are executed,
> and
> > for managing the event bus.  This would get rid of all this
> confusing
> > "inner" and "outer" stuff.  There'd just be lists of things, and
> rules
> > for how you iterate over those lists.  Way easier to understand.
> 
> The "inner" and "outer" come from the decorator pattern. Each trait is
> a decorator around the one which is inner with respect to it. What
> you're
> proposing would be a fundamental change to how traits work.
> 


It sounds like you agree with most of the points I raised, but this is
the one you remain unconvinced of.  It's hard to articulate why I think
this is so important and why I don't think it's as radical a change as
it might sound.

To be clear, what I'm suggesting is to remove keyEvent() from Decorator
and instead just have myKeyEvent() on the pieces.  Centralizing how key
events are issued to traits in a single place will give you two
benefits.  First it will be clear what the visiting algorithm is (rather
than needing to go into every trait to find out) but secondly, and more
importantly, it will force the visiting algorithm to be consistent.  I'm
talking about separating concerns.  Have the trait only be concerned
with the action it needs to execute.  Have the trait visitor be
concerned with stuff like what order traits should be visited in under
what circumstances, when one trait should block another trait etc.  This
would not only make existing behaviour more consistent, it would force
future behaviour to be consistent too.

Ken

_______________________________________________
Read this topic online here:
http://www.vassalengine.org/forum/viewtopic.php?p=19038#p19038


More information about the messages mailing list