Discussion:
[Freecol-developers] developing FreeCol in Eclipse - minor cleanup
Petr Fišer
2016-05-01 19:04:01 UTC
Permalink
Hi guys,
It is a long time since I participated on the project but I would like
to jump in again. :)

I'm pretty used to Eclipse IDE but when I tried to get git.1756c04
working, IDE complained about type safety on some things in
ChooseFoundingFatherMessage.java.

I fixed this by adding necessary generics to static method calls and
tweaked few other lines a bit and IDE build now works which could be,
in turn, more pleasant for other developers.
Quick check and two played games did not show any errors, so I made it
to a patch.

Would you mind to take a look at it?

Thanks in advance,
Fiisch
Michael T. Pope
2016-05-04 08:51:34 UTC
Permalink
On Sun, 1 May 2016 21:04:01 +0200
Post by Petr Fišer
I'm pretty used to Eclipse IDE but when I tried to get git.1756c04
working, IDE complained about type safety on some things in
ChooseFoundingFatherMessage.java.
Your patch is clearly correct. OTOH, Eclipse is clearly wrong to require
it as the java compiler has no difficulty with the original code. I broke
up the expressions a bit in git.e1d318a, can you see if that satisfies Eclipse?
I would prefer not to break the convention of always using a static import
of modules from .../common/utils.

Cheers,
Mike Pope
Petr Fišer
2016-05-05 16:01:26 UTC
Permalink
In Eclipse, the rewrite git.e1d318a looks okay for the IDE.
I must admit, this version is more readable than mine or the original one.
Thank you, Mike

Fiisch
Hi,
Keeping the convention is definitelly a valid reason and I would
rather use another IDE than make you to break code style.
Perhaps, this is more a case for Eclipse devs to tweak up their code helper. :)
The main thought behind the patch was to ease development for others
and Eclipse was a good choice. Perhaps IntelliJ could do the trick.
What do you use?
I'll check git.e1d318a when I get home.
Regards,
Fiisch
Post by Michael T. Pope
On Sun, 1 May 2016 21:04:01 +0200
Post by Petr Fišer
I'm pretty used to Eclipse IDE but when I tried to get git.1756c04
working, IDE complained about type safety on some things in
ChooseFoundingFatherMessage.java.
Your patch is clearly correct. OTOH, Eclipse is clearly wrong to require
it as the java compiler has no difficulty with the original code. I broke
up the expressions a bit in git.e1d318a, can you see if that satisfies Eclipse?
I would prefer not to break the convention of always using a static import
of modules from .../common/utils.
Cheers,
Mike Pope
Michael T. Pope
2016-05-05 22:16:19 UTC
Permalink
On Thu, 5 May 2016 12:47:37 +0200
Keeping the convention is definitelly a valid reason and I would
rather use another IDE than make you to break code style.
That is good of you, but I think it is the wrong policy. Eclipse may be
at fault here, but I do not think a coding convention should be allowed to
throw up barriers to tool use. I would like to see FreeCol not throw any
sort of (major:-)[1] warning with *any* credible tool.
Perhaps, this is more a case for Eclipse devs to tweak up their code helper. :)
I think they may have a problem with their type inferencing, yes.
The main thought behind the patch was to ease development for others
and Eclipse was a good choice.
Agreed.
What do you use?
Emacs + javac. If I want more warnings[1] I use findbugs, and I have
registered FreeCol for the free coverity scans. Those two tools have
prompted some race condition cleanups, but there is a lot of noise.
In Eclipse, the rewrite git.e1d318a looks okay for the IDE.
Excellent. I think that is the right way to resolve these difficulties.
I must admit, this version is more readable than mine or the original one.
The new routines in CollectionUtils are still evolving. I admit
"transform" can be a bit obscure until you have used it 20 times, which
FreeCol does, easily. Be glad you did not have to read the pure Java 8
Streams version!

Cheers,
Mike Pope

[1] There are still several compiler warnings that we explicitly
suppress (@SuppressWarnings). It would be really nice to clean them up,
but I fear all the easy ones are done.
Petr Fišer
2016-05-08 07:47:56 UTC
Permalink
Post by Michael T. Pope
Perhaps, this is more a case for Eclipse devs to tweak up their code helper. :)
I think they may have a problem with their type inferencing, yes.
I checked Eclipse bugzilla. It should be fixed in the current (Eclipse
Neon) release.
Post by Michael T. Pope
Be glad you did not have to read the pure Java 8 Streams version!
I have read the specs. Had no problem with understanding, but being primarily a
sysadmin and not a developer, I decided there are some things in the world
that I just do not need to know. :D
Post by Michael T. Pope
[1] There are still several compiler warnings that we explicitly
suppress (@SuppressWarnings). It would be really nice to clean them up,
but I fear all the easy ones are done.

The code assistant in IDE also found some pieces in dead code
(basically some unreachable if somewhere and such). I could look into it,
or if more desirable, would take some bug to solve.

Fiisch
Post by Michael T. Pope
On Thu, 5 May 2016 12:47:37 +0200
Keeping the convention is definitelly a valid reason and I would
rather use another IDE than make you to break code style.
That is good of you, but I think it is the wrong policy. Eclipse may be
at fault here, but I do not think a coding convention should be allowed to
throw up barriers to tool use. I would like to see FreeCol not throw any
sort of (major:-)[1] warning with *any* credible tool.
Perhaps, this is more a case for Eclipse devs to tweak up their code helper. :)
I think they may have a problem with their type inferencing, yes.
The main thought behind the patch was to ease development for others
and Eclipse was a good choice.
Agreed.
What do you use?
Emacs + javac. If I want more warnings[1] I use findbugs, and I have
registered FreeCol for the free coverity scans. Those two tools have
prompted some race condition cleanups, but there is a lot of noise.
In Eclipse, the rewrite git.e1d318a looks okay for the IDE.
Excellent. I think that is the right way to resolve these difficulties.
I must admit, this version is more readable than mine or the original one.
The new routines in CollectionUtils are still evolving. I admit
"transform" can be a bit obscure until you have used it 20 times, which
FreeCol does, easily. Be glad you did not have to read the pure Java 8
Streams version!
Cheers,
Mike Pope
[1] There are still several compiler warnings that we explicitly
but I fear all the easy ones are done.
Michael T. Pope
2016-05-09 08:27:24 UTC
Permalink
On Sun, 8 May 2016 09:47:56 +0200
Post by Petr Fišer
The code assistant in IDE also found some pieces in dead code
(basically some unreachable if somewhere and such). I could look into it,
or if more desirable, would take some bug to solve.
I have been slack about dead code removal, but got around to checking it
today, and cleaned up a few things in git.51539cf.

Cheers,
Mike Pope

Loading...