Github user sebbASF commented on the issue:

    https://github.com/apache/commons-lang/pull/231
  
    I wonder why the architecture String constants are not an enum?
    
    Also, the various addxxx() methods don't check if there is already an 
entry, so it's possible to overwrite an existing value. This should be easy to 
fix (I suggest a common method to update the map called by all the others).
    
    ===
    
    The current design assumes that a given OS string can only be in one of the 
classifications.
    Will that always be the case?
    At present checking for 64bit JVMs means knowing which classes are 64 bit 
etc.
    It would be possible (but awkward) to add a method for isIntel.
    Perhaps consider using separate flags for each attribute, instead of names 
for specific combinations of attributes. This would make it much easier to add 
new attributes, e.g. CISC,RISC


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to