You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "asbachb (via GitHub)" <gi...@apache.org> on 2023/05/20 21:10:26 UTC

[GitHub] [netbeans] asbachb commented on pull request #5970: Refactoring: Simplify `Profile`

asbachb commented on PR #5970:
URL: https://github.com/apache/netbeans/pull/5970#issuecomment-1556007018

   > to avoid the risk of someone adding an enum without updating the bundle properties we can inline the props into the class, consider removing Bundle.properties and applying this (includes changes from other comments):
   > 
   > ```diff
   > diff --git a/enterprise/j2ee.core/src/org/netbeans/api/j2ee/core/Profile.java b/enterprise/j2ee.core/src/org/netbeans/api/j2ee/core/Profile.java
   > index e16ae8b..f103a9f 100644
   > --- a/enterprise/j2ee.core/src/org/netbeans/api/j2ee/core/Profile.java
   > +++ b/enterprise/j2ee.core/src/org/netbeans/api/j2ee/core/Profile.java
   > @@ -24,6 +24,7 @@
   >  import org.netbeans.api.annotations.common.NonNull;
   >  import org.netbeans.api.annotations.common.NullAllowed;
   >  import org.openide.util.NbBundle;
   > +import org.openide.util.NbBundle.Messages;
   >  
   >  /**
   >   * Represents the defined Java EE profiles.
   > @@ -34,48 +35,71 @@
   >  
   >      // !!! ATTENTION: BE AWARE OF THE ENUM ORDER! It controls compatibility and UI position.
   >      // Do not ever change name of this constant - it is copied from j2eeserver
   > +    @Messages("J2EE_13.displayName=J2EE 1.3")
   >      J2EE_13("1.3"),
   > +
   >      // Do not ever change name of this constant - it is copied from j2eeserver
   > +    @Messages("J2EE_14.displayName=J2EE 1.4")
   >      J2EE_14("1.4"),
   > +
   >      // Do not ever change name of this constant - it is copied from j2eeserver
   > +    @Messages("JAVA_EE_5.displayName=Java EE 5")
   >      JAVA_EE_5("1.5"),
   > +
   > +    @Messages("JAVA_EE_6_WEB.displayName=Java EE 6 Web")
   >      JAVA_EE_6_WEB("1.6", "web"),
   > +
   > +    @Messages("JAVA_EE_6_FULL.displayName=Java EE 6")
   >      JAVA_EE_6_FULL("1.6"),
   > +
   > +    @Messages("JAVA_EE_7_WEB.displayName=Java EE 7 Web")
   >      JAVA_EE_7_WEB("1.7", "web"),
   > +
   > +    @Messages("JAVA_EE_7_FULL.displayName=Java EE 7")
   >      JAVA_EE_7_FULL("1.7"),
   > +
   > +    @Messages("JAVA_EE_8_WEB.displayName=Java EE 8 Web")
   >      JAVA_EE_8_WEB("1.8", "web"),
   > +
   > +    @Messages("JAVA_EE_8_FULL.displayName=Java EE 8")
   >      JAVA_EE_8_FULL("1.8"),
   > +
   > +    @Messages("JAKARTA_EE_8_WEB.displayName=Jakarta EE 8 Web")
   >      JAKARTA_EE_8_WEB("8.0", "web"),
   > +
   > +    @Messages("JAKARTA_EE_8_FULL.displayName=Jakarta EE 8")
   >      JAKARTA_EE_8_FULL("8.0"),
   > +
   > +    @Messages("JAKARTA_EE_9_WEB.displayName=Jakarta EE 9 Web")
   >      JAKARTA_EE_9_WEB("9.0", "web"),
   > +
   > +    @Messages("JAKARTA_EE_9_FULL.displayName=Jakarta EE 9")
   >      JAKARTA_EE_9_FULL("9.0"),
   > +
   > +    @Messages("JAKARTA_EE_9_1_WEB.displayName=Jakarta EE 9.1 Web")
   >      JAKARTA_EE_9_1_WEB("9.1", "web"),
   > +
   > +    @Messages("JAKARTA_EE_9_1_FULL.displayName=Jakarta EE 9.1")
   >      JAKARTA_EE_9_1_FULL("9.1"),
   > +
   > +    @Messages("JAKARTA_EE_10_WEB.displayName=Jakarta EE 10 Web")
   >      JAKARTA_EE_10_WEB("10", "web"),
   > +
   > +    @NbBundle.Messages("JAKARTA_EE_10_FULL.displayName=Jakarta EE 10")
   >      JAKARTA_EE_10_FULL("10");
   >      // !!! ATTENTION: BE AWARE OF THE ENUM ORDER! It controls compatibility and UI position.
   >  
   > -    public static final Comparator<Profile> UI_COMPARATOR = new Comparator<Profile>() {
   > -
   > -        @Override
   > -        public int compare(Profile o1, Profile o2) {
   > -            return -(o1.ordinal() - o2.ordinal());
   > -        }
   > -    };
   > +    public static final Comparator<Profile> UI_COMPARATOR = (Profile o1, Profile o2) -> -(o1.ordinal() - o2.ordinal());
   >  
   >      // cache
   >      private final String propertiesString;
   >  
   >      private Profile(String canonicalName) {
   > -        this(canonicalName, null);
   > +        this.propertiesString = canonicalName;
   >      }
   >  
   >      private Profile(String canonicalName, String profile) {
   > -        StringBuilder builder = new StringBuilder(canonicalName);
   > -        if (profile != null) {
   > -            builder.append("-").append(profile); // NOI18N
   > -        }
   > -        this.propertiesString = builder.toString();
   > +        this.propertiesString = canonicalName + "-" + profile;
   >      }
   >  
   >      /**
   > @@ -132,7 +156,7 @@
   >          for (Profile profile : Profile.values()) {
   >              if (profile.toPropertiesString().equals(valueMinusQuotes)
   >                      || profile.name().equals(valueMinusQuotes)
   > -                    || profile.name().endsWith(valueMinusQuotes)) {
   > +                    || (valueMinusQuotes.startsWith("EE_") && profile.name().endsWith(valueMinusQuotes))) {
   >                  return profile;
   >              }
   >          }
   > ```
   
   What do you think about having a test validating that the `Bundle.properties` contains everything of the enum. With your approach we still cannot ensure that somebody will use an incorrect naming, when we have a test we can ensure that and prevent nothing being rendered because of an Exception.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists