converted to 1.6 markup
|No differences found!|
This page contains the full discussion of item DJD1 in the SharedComponentVersioningGuidelinesReview.
DJD: Don't force commonality just because classes or concepts seem to be somewhat similar.
JNB: PVH is overkill but an int is too simple. I'd rather see a feature mechanism. We should be concerned about coupling to common due to the versioning mechanism
Jeremy then sent an email to clarify:
I mentioned in SharedComponentVersioningGuidelinesReview about a feature mechanism rather than version numbers and thought I should probably should explain that more.
The concern I had with version numbering is that there is a lot of implicit information locked up in a single String or int. As the objective of this is to cater for scenarios where there is skew between consumer and implementation I think it is better to make this more explicit.
To that end, I was envisioning a mechanism where the consumer could easily check if a specific feature was provided by the implementation rather than seeing which version was there and then inferring.
To achieve this, I was thinking the primary interface would have a method like:
This needs to be loosely coupled i.e. featureId Strings would be documented but the consumer could not rely e.g. on a constant field as that field may not be defined by earlier versions.
DVC: I think you have a good point, Jeremy. The consumer shouldn't have to calculate what features are available based on a version number. This logic should be encapsulated in the module, not distributed across all the consumers.
I disagree however that the parameters have to be Strings. Integer constants are compiled down to simple integers so there will not be a runtime error. I would prefer integers constants because it avoids mistakes (which would be very hard to detect, you would just get a "false" when it should be "true") and allows for faster lookup.
Here is an example:
is compiled down to something like
If it's the old implementation of CommonModule, it will see an integer value it doesn't recognize (either because the class or the feature does not exist) and return false.
In terms of the conditional calling of a method that may or may not exist in the class, I'd like to make sure that you can actually do this without getting an error. Craig Russell was telling me he wasn't sure whether you could have code blocks that call non-existent methods even if the actual method call is never hit due to conditional logic. I'm going to do some testing to see how that pans out.
JNB: You learn something every day - I thought the constants were resolved at link time but they are added to the constant pool. This would mean that internalized Strings could also be used which would compare as quickly as ints but have more meaning to the user.
To answer Craig's concern, I believe you can. Strictly, you don't need the check at all, you just get a NoSuchMethodError if you try to invoke a method that does not exist (which you could catch and ignore but I think that's ugly).
DVC: I moved Tomohito's comments as separate items in SharedComponentVersioningGuidelinesReview as I felt they were separate topics from the discussion of version detection and how to decide if a feature is available.
DVC: Jeremy, can you explain why internalized Strings have more meaning than ints? In the code whether they're strings or not they'll be declared as a constant and used as such in the code.
So whether you have public static final String FANCY_FEATURE = "FancyFeature" or public static final int FANCY_FEATURE = 2 it still looks the same in the calling code, e.g. if CommonModule.implementsFeature(CommonModule.FANCY_FEATURE). I prefer ints because then I could do a simple array and look up by index, and determine if a feature is not implemented by noticing that the index is out of range.