Github user miguelaferreira commented on the pull request:
https://github.com/apache/cloudstack/pull/778#issuecomment-140493836
I do agree that prepending "s_" to static variables is a waste of
characters. Modern IDEs will signal static variables for you. It's a bit like
prepending (or appending) type information in the variable name. There was a
time it was useful, but that time is long gone.
I also appreciate abstraction in code, that is, you've now introduced a
protected variable in a base class that will be available to all the classes
that extend it. However I do not see a real benefit in adding this particular
indirection level. I mean, loggers are very straight forward to add and
understand, so standardising on they declaration and use, seem to me far better
than having it be a static variable in some classes, and a non
static-protected-inherited variable in others.
There is plenty I would like see done in terms of cleaning the code of ACS.
For instance I've measured duplicate code a year ago and found 25% of the code
to be redundant.
(http://www.slideshare.net/miguel_f/20141121-cccenoanimations)
The DB layer is another example. ACS is at the moment using a deprecated
library for it's DB layer, while there are other libraries (e.g. spring-data)
that would actually generate most of the hand-made code we have now.
I'll be very happy to share my thoughts with you about potential
refactorings that would benefit the ACS code.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
