Refactor the ENUMs to remove all the unnecessary looping
12029
Reporter: trobertson
Type: Improvement
Summary: Refactor the ENUMs to remove all the unnecessary looping
Priority: Trivial
Resolution: Fixed
Status: Closed
Created: 2012-10-15 16:03:32.735
Updated: 2016-10-06 13:42:00.743
Resolved: 2016-10-06 13:42:00.61
Description: Please look at the implementation of the Kingdom Enum
http://code.google.com/p/gbif-common-resources/source/browse/gbif-api/trunk/src/main/java/org/gbif/api/vocabulary/Kingdom.java
This, like most of our ENUMs seems to me to have unnecessary looping at runtime, and also makes use of ordinal (eek).
Secondly, people are doing things like this in other code
private static final List KINGDOMS = ImmutableList.of(Kingdom.ANIMALIA, Kingdom.ARCHAEA, Kingdom.BACTERIA, Kingdom.CHROMISTA,
Kingdom.FUNGI, Kingdom.PLANTAE, Kingdom.PROTOZOA, Kingdom.VIRUSES, Kingdom.INCERTAE_SEDIS);
Please see the proposed patch which I believe to be a better implementation.
]]>
Author: lfrancke@gbif.org
Created: 2012-10-15 16:25:05.262
Updated: 2012-10-15 16:25:05.262
In general I think your idea is a good one but I think your implementation could be better. This way it's easy for anyone to forget to update the Map.
The current code could be used to pre populate a map in a static block. I'll add a patch of what I mean shortly.
Author: trobertson@gbif.org
Comment: Agree, second patch is better implementation.
Created: 2012-10-15 20:32:06.347
Updated: 2012-10-15 20:32:06.347
Author: lfrancke@gbif.org
Comment: While we're at it we might just as well change {{nubUsageID}} to {{nubUsageId}}.
Created: 2012-10-15 20:43:21.6
Updated: 2012-10-15 20:43:21.6
Author: trobertson@gbif.org
Created: 2012-10-16 10:18:59.294
Updated: 2012-10-16 10:18:59.294
[~mdoering@gbif.org] I'm not sure if it was you who coded this stuff originally, but I think it might have been.
What do you think to this proposal? I'm not suggesting we do this now, but might help guide further developments.
Author: mdoering@gbif.org
Created: 2012-10-16 10:41:58.183
Updated: 2012-10-16 10:41:58.183
Yes, that should have been me, copied over from the old ecat codebase.
The patch looks fine, only improvement I can see is using the abbreviation property to populate the hashmap instead of another literal char.
In the long term these abbreviation will not be used anymore though I guess - they are in use in the deprecated ecat webservices.
Author: lfrancke@gbif.org
Comment: Thanks for commenting Markus. Have you maybe looked at the older patch? Otherwise I don't understand the part about the literal char. If we could deprecate the abbrev stuff that'd be even better but would be a different issue I suppose.
Created: 2012-10-16 10:46:11.427
Updated: 2012-10-16 10:46:11.427
Author: mdoering@gbif.org
Created: 2012-10-16 12:13:57.681
Updated: 2012-10-16 12:13:57.681
Indeed, I just looked at the one at the bottom which was the old one. The new one looks very fine!
We could deprecate the abbreviation, but unless we close down the old webservices we gotta live with them still - and thats probably still for quite a while