Re: [Rails-devel] Comments on Patch for Disclaimer, UserConfirmation and new Main-Class

[Rainer M. <rai...@we...>]

>My comments to some of the points raised by Rainer.=20
>=20
>On the code:=20
>=20
>It's not easy to read the kind of patch you have sent,=20
>but from the part on LocalText I understand that you have=20
>added a dummy constructor and also have factored out
>getLocalisedText().=20
>=20
>To me this is entirely a matter of taste, about which I'm not going
>to=20
>argue,=20
>but I *would* argue that both should be private or protected rather
>than=20
>public.=20
I have need the LocalizedText to get the language. There are =
instance-ethods
to get it, but since you never instanciated the class, I could not used =
it.
Since I try to avoid a complete refatoring, I used this work-a-round
instead.


>=20
>When creating a server (see below) we might want to create an
>instance=20
>(or a Properties map) per locale, but even then all of this should
>IMO be=20
>managed internally.=20
>
So a german and an japanese user could not play together? - no good =
idea.
But I agree with your encapsultion thoughts. If you give me a change I =
hope
I can show you what I whould prefer.


>=20
>> - getLogger()=20
>> Can be also in Util. I put it in here, because it is a=20
>> rails-spezifig way=20
>> to log.=20
>> Find the Logger over a Method makes it easiar to change the
>fist=20
>> desission to log on package-level.=20
>=20
>Sorry, I don't understand at all what you are trying to say here.=20
When you start to introduce log4j in devel-rails, you talk about the
posibility to create a logger for each class (or even instance),
but for rails, it should be enough, to have a logger at package-level.

To create a logger on a defined way would be a good think, I thought. So =
why
writting at each class the same code to get the package name and create =
a
logger,=20
if we can write a central method, with just need the class or an =
instance of
it, to get the logger?


>=20
>> - VERSION=20
>> Final variables should be written in upper letters
>(Sun-Conversion)=20
>=20
>I thought that *constants* should be written in upper case.=20
>The question is whether 'version' is really a constant.=20
>One can also see it as a variable that, once assigned, cannot be
>changed -=20
>but the value is different in each release.=20
>=20
>But if you say that this is nitpicking, I agree,=20
>and I have no real problem with making it upper case.=20
>
It was just an example. If you look at the code, you find diffrent =
styles
and naming-praxis, with makes it harder to understand it.
Writing constants in upper letter is just one think, with should know =
most
coders and most coders follows.
Anyway, when I will come to the project, you will see that I will make =
such
easy avoidable mistakes also. - don't bother.


>> - Config (comment)=20
>> - Much better not as static class, So you can have userConfig
>and=20
>> systemConfig. Both should than accessed by RailsSystem or maybe
>better=20
>> Util-Methods=20
>=20
>I was thinking it would be easier to have all (user and system)=20
>properties in one Properties map, so that the programmer need=20
>not worry about where a property is exactly defined.=20
>We could then also easily move properties between property files=20
>if that would suit us better.=20
>=20
>We definitely need static access to properties, otherwise we'll=20
>have to push around another instance reference.=20
>I have no real problem to replace Config.get() by Util.getConfig(),=20
>but I don't see the benefit either. To me, it's just an extra layer=20
>of indirection.=20
>
If we need system.properties we should not put it into the =
user.properties,
becouse the user should normaly not see it or change it.
Right know, there is no need for system.properties, and Right know, the =
is
no need for more then one user.properties per instance.
(which will be changing on a server-base-version).
But things can change, and one think, I have learnd on the hard way was,
that it is very hard to remove static access with multible access.
So I try to avoid static und public access where ever I can. You will =
find
this guidlines in many books, but I have not read an explanation for the
first one (avoid static).
If we will someday on a teamspaek talk, I will tell you my hard learning =
(to
long to write it).


>> - Where do you define the default-values? (if the file is=20
>> coruppted by=20
>> the user)=20
>=20
>Where needed, defaults are specified where the properties are used=20
>when they turn out to be null or empty.=20
>=20
So if you need the default more then ones, you defined it more then =
ones? -
Bad idea.


>Not every property has an explicit and fixed default value.=20
>The default money amount format is game-dependent.=20
>The default language is (British) English, because the default=20
>resource bundle is written in that language.=20
>The default log4j properties are (by definition) empty.=20
>Etc.=20
If there are no defaults - OK, but if there are, why not using a
property-file for it too.
the resource bundle has a default one (with no language extension).


>=20
>> - CodeStyle (comment)=20
>> - public and protected variables: You are much to=20
>> broadminded with it.=20
>> You should capsulate as much as possible, using getter- and=20
>> setter-methods=20
>> for access.=20
>=20
>One problem I have with Java is, that you cannot distinguish=20
>between variables that may only be visible to subclasses,=20
>and variables that may be visible to all classes within a package.=20
>Both are "protected".=20
- protected: means only for subclasses
- private: only for class and private or anonym classes in the same file
- 'none': package wide with does not include subclasses in other =
packages


>In my code in this project, most or all "protected" variables are
>meant=20
>to be used in subclasses only, even where such subclasses do not yet
>exist.=20
>We expect that a lot of subclassing will be needed in the future=20
>when we are going to add dissimilar games.=20
>So I am only prefixing instance variables with "private"=20
>where I am pretty sure that these will never be used in subclasses.=20
>=20
>The rule I use, is that subclasses have free access to its
>superclass=20
>variables without need to use a getter. But non-related classes=20
>in the same package should always use getters and setters.=20
>=20
>BTW I think that the 'game' package is getting=20
>too large, and that we might need to split it up.=20
>That would have the side effect of better "protection" too.=20
>=20
>I agree that in principle there should not be public variables.=20
>Do we really have sinned as much as you seem to imply?=20
>It is possible that I have not always been consistent,=20
>so I would appreciate your examples of wrong use of public etc.=20
For example, all windows (views) are public, so I guess you call them =
direct
from the gmae-classes (model).
That will make it imposible to write diffrent views. If you have =
methods,
you can write interfaces someday and call the methods over the =
interface.
At this way you do not know the concret implementation of the view.


>=20
>And (hmmm) see my comments on your version of LocalText above....=20
>=20
>> - static: Much to broadminded also. You should avoid=20
>> static as much as=20
>> possible. (static constants are a break in the rule)=20
>=20
>As a matter of principle? I don't see why.=20
>To me, this is very project-dependent.=20
>=20
>> You will get a lot of trouble to bring this code running in a=20
>> server-environment. No chance.=20
>> Even singletons are not very good on servers (but easy to=20
>> transform).=20
>> RailsSystem as a geneneral System-API-Class and Util for general=20
>> transformation and methods should be the only Classes which=20
>> have static=20
>> methods which are not factory methods or system-wide-parameters=20
>> Hard words but I have a bad feeling for the time this=20
>> project groose more=20
>> complex and it will be nearly imposible to have a=20
>> server-side-version with=20
>> multible games with this code.=20
>> Start smart is a good thing, but some day, you have to=20
>> redesign it and if=20
>> you go this way farther it will be imposible someday - sorry.=20
>=20
>I am fully aware that a lot of work needs be done=20
>to make a multi-game server out of this code.=20
>If we ever get that far.=20
>=20
>The problem is, that if you don't have class methods,=20
>you need at least some reference variables in many parts=20
>of the code to access widely used instances.=20
>=20
>Several parts of the code need to be able to find out in which turn,=20
>round or phase the game is (this all may have changed=20
>any moment, e.g. while processing a train buy).=20
>So there must be some central point of inquiry about all=20
>aspects of the game state.
A central controller and interfaces


>=20
>I am fully aware that the mixed use of class and instance variables=20
>and methods is a mess in some places (e.g. Game/GameManager),=20
>and that this needs to be redone in a better way.=20
>However, in my opinion the code is not yet in a sufficiently=20
>mature state that we can take decisions on exactly how to do that.=20
>=20
>I suppose the best way will be to make sure that every=20
>instance that must take decisions has a reference to an=20
>instance of either Game or GameManager, and can find out=20
>the current status by calling that instance's methods.=20
Very good idea.

>=20
>I don't think this will be very difficult to do.=20
>So far I did not have the feeling that this is more urgent=20
>than improving the code structure in more fundamental ways,=20
>to enable undo/redo and to prepare a client/server split,=20
>which is what I have been doing the last few months, and still do.=20
>=20
>=20
>Aside: I sometimes take shortcuts to use the little bits of time=20
>I have as productively as possible.=20
>And quite often I come back (much) later to that code=20
>and fix it, or improve it in some other way, or entirely=20
>remove it or replace it with something better.=20
>There is quite some code that I know must be improved,=20
>or replaced, or whatever - but often I just have to postpone=20
>my thinking on what exactly needs be done.=20
>In my experience, things usually become clear when the time is ripe.=20
Thats the way I have coded in the 80's. On a very large project (and I =
think
this is a large project - at least what the final vision is),
I was stucked in the code, with becomes unchangeable and had to start =
from
the beginning again,=20
That was the time, I start with uge modelling and tryed to defnie every
posibility in forcast. Good think, but much wast time for thinks, which =
will
never come.
There are some articels about 'the entrophy of code change', with meens,
that every change or add on code makes it harder to do the next change =
or
add on.
Agile proofs, that with good, very object oriented programm style, you =
can
drop the entrophy near zero. But you have to seperate your classes as =
far as
possible.
I hope, that with aspect programming, this will go even better, but that =
is
another story.
Today I try to define the vision goals and make models just for the next
step. Try to be agile by implementing as smal as possible,=20
BUT thing about the vision goals on every implementation step. Doing =
thinks,
with pure agile peoble would posible not do.


>=20
>> I know, my code is not a good example how it shoud be=20
>> (since it is not=20
>> much code at all and most of it in RailsSystem - the=20
>> exception of the rule,=20
>> Maybe I can refactor LocalText or Config to show you what=20
>> I mean, if you=20
>> want it.=20
>=20
>Showcases are always welcome to me.=20
>It is from code from other people that I learn,=20
>even if I adopt nothing or just a little of their style.=20
>I'm sure it's the same with you.=20
>=20
>Sorry that this reaction has become so long,=20
>but I hope I have been able to clarify a few aspects.=20

You are welcome. It makes thinks much more clear. I guess, that we will =
have
a good time together.
And please remind: Even if I have more expirence - Fresh thinking is the
way, we move on.
Even if I critisize a lot, the general ideas are great and changes have =
to
be made step by step.
I would not like to refaktor evrythink without going on implementing new
thinks also.

>- Questions=20
> - Java-Version=20
> Which one are you using? By know I have used only 1.4.2
>syntax, but=20
>would like to go up to 1.6 (or 1.5).=20
> 1.5 should be faster and has more type-safty and a
>enumaration.=20
> 1.6 should be faster then 1.5 and has some advantages in=20
>web-services=20
> At work, we are using still 1.4.2, becouse we have some=20
>third-party-libs, which are not compatible with 1.5, but I would
>like to get=20
>experince in 1.5/1.6=20
>=20
>For the sake of it? That does not sound like a good reason to start
>using it in this project.=20
>I don't know much about Java 1.5, so I would like to know why we
>would need it in this project.=20
>=20
>=20
>The last time we talked about it (at least 1 or 2 years ago), we had
>decided to use Java 1.4.2. However, that might be something we can
>revisit. I'm not sure how Java's adoption has been going. If most
>people have 1.5 or 1.6, I don't see a problem changing our system
>requirements.=20
>=20
>If there is a clear need, we should upgrade, but I don't see it
>yet.=20
>If it is speed, we can always recommend people to use the latest=20
>and fastest version of java, without relying upon it for our source
>code.=20
>=20
Somebody else said, 1.6 is still in beta. I was at the javasoft.com page =
and
could see that, but have not download it I'm not sure.

We don't need 1.5. You can do all thinks with 1.4.2.=20
As long as we do not use some knew protected key words of 1.5 (enum for
example),
it will be very simple to move up as time comes.
It was just a thought, becouse I have used 1.5 and eclipse told me, that =
a
method I used is depredicated at 1.5.
And since I like to be up to date and learn what is new, I asked.
(Spezialy I like to get experience in the =
type-save-collection-class-using
and the enums).

There was an article about the improvments of 1.6 in the german computer
magazine c't.=20
Since it makes web-services-implementation and use much easear, I guess,
that a change could be intristing, when we come to that point.
If 1.6 is still in beta, I would not use it.

Rainer