Re: [Rails-devel] Hex trains support

[Erik V. <eri...@xs...>]

> -----Original Message-----
> From: Stefan Frey [mailto:ste...@we...]
> I have only checked the methods that train and TrainType support instead
of
> having a look at the xml files ;-)
> 
> I intend to keep it as closely to your current specification. So I agree
with your
> proposal.  Considering your specification I only wonder why major and
minor
> are an attribute of the TrainType tag instead of Reach?
> Maybe to keep the xml files more compact?

That might be part of the answer, but I think the real reason was very
practical: the number of stops cannot be defaulted, whereas the <Reach> and
<Score> attributes usually are identical for all trains and so only a
game-wide default value is needed. 

Surely it is not very consistent this way: I could as well have added 'base'
and 'countTowns' etc. as attributes to a <DefaultType> tag.  I think I
created <Reach> and <Score> as separate tags because I felt the attributes
of these tags could better be split than live together as attributes of just
one tag.

I'm not against aligning these things either way, but I don't consider it
very important.


> The only change will be that there is an additional hexes attribute inside
of
> the TrainType tag, which should be used for hex trains.
> 
> This attribute allows the separation of name and range (thus the hex train
in
> 1844 will be defined with a hexes attribute one less than their name).
> 
> Some implementation questions:
> 
> The main task is to add methods to support the variables and I will to
that.
> Just to be sure I understood your recent changes:
> 
> TrainCerticate: One for each certificate
> 
> TrainType: One for each type

Correct so far.

> Train: One per train certificate (standard case), two per train
certificate (dual
> case)

No, one train per certificate.  Its initial type is null, will be set if the
train is bought, and unset if it is discarded to the Pool.

> Train objects have a pointer to the TrainType object, which itself
contains all
> relevant attributes required for the revenue calculation.

... if not null.

> For the case of operating I can safely ignore the dual case as the
operating
> company has the correct trains in its portfolio.
> 
> So I retrieve all trains for a company from the portfolio and for each
train get
> its TrainType and then query the TrainType for the attributes.
> 
> To make this more transparent I suggest to remove all methods related to
> range etc. from the Train class (and TrainI interface) and only keep those
in
> TrainType. I currently sometimes query the Train and sometimes the
> TrainType and this seemed somehow inconsistent.

Agreed.

Erik.