[messages] [Developers] Vassal code maintainability
uckelman at nomic.net
Fri Jul 30 02:59:39 MST 2010
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.
> 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
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. When I get back to my Model-
View project, that will involve ripping out a huge amount of old code,
at which point I intend to write tests for the new code.
If you want to write tests, please do. We would gladly accept them.
> 3. Layering. Why are there awt imports on model classes like
> TriggerAction and GamePiece. Has anyone ever made an attempt to bring
> some Model-View-Controller discipline to the Vassal code base? I think
> this mixing of model and view is part of the reason we have strange
> things like keystrokes being the way you call a function in Vassal,
> which is the weirdest thing I've seen in a while!
See my comment above. The way these things are mixed together right now
is a catastrophe. This is something I've wanted to fix almost since I
started working on VASSAL. If you look at our Development Roadmap,
you can see this stuff under 3.3:
> 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.
> Those are four things off the top of my head that I feel would help
> improve the maintainability of the code and reduce the chance of code
> changes introducing bugs in the future. Has anyone else considered
> introducing disciplines like these to the Vassal code base?
Regarding #2 and #3, I'm in complete agreement with you, and have been
for some years now. I would love to have more development help in order
to make these happen. (I would also love to have someone to hand over
the website to, so that I have time to do any development at all again.)
More information about the messages