kriegaex commented on code in PR #125:
URL: https://github.com/apache/xalan-java/pull/125#discussion_r1401430440


##########
serializer/src/main/java/org/apache/xml/serializer/Version.java:
##########
@@ -20,33 +20,97 @@
  */
 package org.apache.xml.serializer;
 
+
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Properties;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 /**
  * Administrative class to keep track of the version number of
  * the Serializer release.
  * <P>This class implements the upcoming standard of having
- * org.apache.project-name.Version.getVersion() be a standard way 
- * to get version information.</P> 
+ * org.apache.project-name.Version.getVersion() be a standard way
+ * to get version information.</P>
  * @xsl.usage general
  */
 public final class Version
 {
+  private static final String POM_PROPERTIES_JAR = 
"org/apache/xml/serializer/version.properties";
+  private static final String POM_PROPERTIES_FILE_SYSTEM = 
"serializer/target/classes" + POM_PROPERTIES_JAR;
+  private static final String VERSION_NUMBER_PATTERN = 
"^(\\d+)[.](\\d+)[.](D)?(\\d+)(-SNAPSHOT)?$";
+  private static final String NO_VERSION = "0.0.0";
+
+  private static String version = NO_VERSION;
+  private static int majorVersionNum;
+  private static int releaseVersionNum;
+  private static int maintenanceVersionNum;
+  private static int developmentVersionNum;
+
+  private static boolean snapshot;

Review Comment:
   The class in `serializer` is final, because I inherited it that way. But 
that can be changed. As it is impossible to override static methods and use 
polymorphism like with instance methods, it is not so super easy to avoid 
duplication 100%. That is why e.g. in class `XSLProcessorVersion extends 
Version` I had to redefine `getVersion()` to make sure that it calls its own 
`getProduct()` method and not the one from its superclass. Again, a singleton 
pattern would be nicer here, but I am not going to do that in this PR. I want 
to port something, not completely refactor it. Plus, the static methods are 
public API, we would have to keep them and make them call something like 
`getInstance().myMethod()`. Then again, we would have the problem that in a 
subclass for the static methods there would not be any polymorphism, and the 
static method `getInstance()` or static field `INSTANCE` would be pulled from 
the super class. We are not on a green field here, we are dealing with public 
classes 
 and public static methods.
   
   As for the dependency of `xalan` on `serializer`, it is indeed defined in 
the 2.7.1 POM. But I had checked before on Maven Central and of course used the 
latest version 2.7.3, which has no such POM and therefore no such dependency 
defined. As POMs for Ant projects are always created somewhat synthetically, as 
a total Xalan newbie I had no way of knowing if the currently defined 
dependency in the Maven branch ought to be an optional or fixed dependency. I 
did not bother to analyse the code to find out, because I was focusing on 
porting the version classes only.
   
   So can we not over-engineer this thing here? I already spent an order of 
magnitude more time discussing the PR than to implement it. These cycles are 
lost to more valuable support work I could have done for Joe with his open 
Maven questions.



-- 
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: dev-unsubscr...@xalan.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@xalan.apache.org
For additional commands, e-mail: dev-h...@xalan.apache.org

Reply via email to