###Nitpicks
Nitpicks
Formatting & Use of API:
LOG
has some very strange whitespace there. Why not simply write
private static final Logger LOG = Logger.getLogger(ApplicationConfig.class.getName());
?- The
Logger#log
method's third argument is a vararg. This means that instead of explicitly creating anew Object[]
you can just skip that and call
LOG.log(Level.WARNING, "message", defaultEventServerPort, defaultClientServerPort);
- On that note: I personally prefer to have the first argument of a method call on the same line as the call. Also I don't like closing the parens on a separate line. But that's personal preference, ignore it at your discretion :)
Needless Work:
The code is doing some unnecessary work in the Constructor. For one it uses Integer.valueOf(String)
, which calls Integer.parseInt(String)
and returns a boxed Integer
object that is then unboxed and assigned to a primitive int.
Additionally your isValidNumeric
could possibly benefit from using a touch of Java Streams:
private static boolean isValidNumeric(String v) {
if (v == null || v.length() == 0) {
return false;
}
return v.codePoints().allMatch(Character::isDigit);
}
While this shouldn't give any performance gains, it's IMO significantly easier to mentally process.
###Terminating the Application / UX
Terminating the Application / UX
Terminating your application is a little difficult. For one the ExecutorService
used for the Servers is going out of scope as soon as main
ends. This makes it needlessly hard to cleanly shut down on certain commands.
Additionally I'd expect to be able to pass the path to a configuration file to my executable so I can decide which configuration I want to go with. This has the added benefit of not needing to repack a jar every time I change configuration. This would necessitate a different loading mechanism for the configuration, since it can't be carried over in the static scope. Incidentally this resolves your grievance with the static initializer block as a side-effect :)
###Nitpicks
Formatting & Use of API:
LOG
has some very strange whitespace there. Why not simply write
private static final Logger LOG = Logger.getLogger(ApplicationConfig.class.getName());
?- The
Logger#log
method's third argument is a vararg. This means that instead of explicitly creating anew Object[]
you can just skip that and call
LOG.log(Level.WARNING, "message", defaultEventServerPort, defaultClientServerPort);
- On that note: I personally prefer to have the first argument of a method call on the same line as the call. Also I don't like closing the parens on a separate line. But that's personal preference, ignore it at your discretion :)
Needless Work:
The code is doing some unnecessary work in the Constructor. For one it uses Integer.valueOf(String)
, which calls Integer.parseInt(String)
and returns a boxed Integer
object that is then unboxed and assigned to a primitive int.
Additionally your isValidNumeric
could possibly benefit from using a touch of Java Streams:
private static boolean isValidNumeric(String v) {
if (v == null || v.length() == 0) {
return false;
}
return v.codePoints().allMatch(Character::isDigit);
}
While this shouldn't give any performance gains, it's IMO significantly easier to mentally process.
###Terminating the Application / UX
Terminating your application is a little difficult. For one the ExecutorService
used for the Servers is going out of scope as soon as main
ends. This makes it needlessly hard to cleanly shut down on certain commands.
Additionally I'd expect to be able to pass the path to a configuration file to my executable so I can decide which configuration I want to go with. This has the added benefit of not needing to repack a jar every time I change configuration. This would necessitate a different loading mechanism for the configuration, since it can't be carried over in the static scope. Incidentally this resolves your grievance with the static initializer block as a side-effect :)
Nitpicks
Formatting & Use of API:
LOG
has some very strange whitespace there. Why not simply write
private static final Logger LOG = Logger.getLogger(ApplicationConfig.class.getName());
?- The
Logger#log
method's third argument is a vararg. This means that instead of explicitly creating anew Object[]
you can just skip that and call
LOG.log(Level.WARNING, "message", defaultEventServerPort, defaultClientServerPort);
- On that note: I personally prefer to have the first argument of a method call on the same line as the call. Also I don't like closing the parens on a separate line. But that's personal preference, ignore it at your discretion :)
Needless Work:
The code is doing some unnecessary work in the Constructor. For one it uses Integer.valueOf(String)
, which calls Integer.parseInt(String)
and returns a boxed Integer
object that is then unboxed and assigned to a primitive int.
Additionally your isValidNumeric
could possibly benefit from using a touch of Java Streams:
private static boolean isValidNumeric(String v) {
if (v == null || v.length() == 0) {
return false;
}
return v.codePoints().allMatch(Character::isDigit);
}
While this shouldn't give any performance gains, it's IMO significantly easier to mentally process.
Terminating the Application / UX
Terminating your application is a little difficult. For one the ExecutorService
used for the Servers is going out of scope as soon as main
ends. This makes it needlessly hard to cleanly shut down on certain commands.
Additionally I'd expect to be able to pass the path to a configuration file to my executable so I can decide which configuration I want to go with. This has the added benefit of not needing to repack a jar every time I change configuration. This would necessitate a different loading mechanism for the configuration, since it can't be carried over in the static scope. Incidentally this resolves your grievance with the static initializer block as a side-effect :)
###Nitpicks
Formatting & Use of API:
LOG
has some very strange whitespace there. Why not simply write
private static final Logger LOG = Logger.getLogger(ApplicationConfig.class.getName());
?- The
Logger#log
method's third argument is a vararg. This means that instead of explicitly creating anew Object[]
you can just skip that and call
LOG.log(Level.WARNING, "message", defaultEventServerPort, defaultClientServerPort);
- On that note: I personally prefer to have the first argument of a method call on the same line as the call. Also I don't like closing the parens on a separate line. But that's personal preference, ignore it at your discretion :)
Needless Work:
The code is doing some unnecessary work in the Constructor. For one it uses Integer.valueOf(String)
, which calls Integer.parseInt(String)
and returns a boxed Integer
object that is then unboxed and assigned to a primitive int.
Additionally your isValidNumeric
could possibly benefit from using a touch of Java Streams:
private static boolean isValidNumeric(String v) {
if (v == null || v.length() == 0) {
return false;
}
return v.codePoints().allMatch(Character::isDigit);
}
While this shouldn't give any performance gains, it's IMO significantly easier to mentally process.
###Terminating the Application / UX
Terminating your application is a little difficult. For one the ExecutorService
used for the Servers is going out of scope as soon as main
ends. This makes it needlessly hard to cleanly shut down on certain commands.
Additionally I'd expect to be able to pass the path to a configuration file to my executable so I can decide which configuration I want to go with. This has the added benefit of not needing to repack a jar every time I change configuration. This would necessitate a different loading mechanism for the configuration, since it can't be carried over in the static scope. Incidentally this resolves your grievance with the static initializer block as a side-effect :)