Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Best practices: use final wherever possible, constructor injection, this. access to fields #191

Merged
merged 4 commits into from May 26, 2015

Conversation

krasa
Copy link
Contributor

@krasa krasa commented Apr 22, 2015

Implementation of following best practices:

Every class should be final by default, except classes explicitly designed to be extensible. Unfortunately Spring beans cannot be final - http://stackoverflow.com/questions/2657432/make-spring-service-classes-final
Also method and constructor parameters, field and variables should be final. This is a no-brainer.

It is recommended to use constructor injection instead of field injection to provide better testability and safety against NPE. Every object should be fully initialized upon construction.

Using .this to access fields provide a safety against accidental use of parameter/variable, thus greatly reducing a number of possible bugs..

Supersedes: #153 #172

Resources:
http://whiley.org/2011/12/06/final-should-be-default-for-classes-in-java

@krasa
Copy link
Contributor Author

krasa commented Apr 22, 2015

Also contains some minor correctness fixes, and a context holder for integration of beans with static utility classes - it might be wise to change them into beans in future.

@mikkelee
Copy link

+1

krasa added 3 commits May 16, 2015 20:10
Conflicts:
	src/main/java/com/seriouscompany/business/java/fizzbuzz/packagenamingpackage/impl/factories/LoopComponentFactory.java
@emiln
Copy link
Member

emiln commented May 26, 2015

I think this is a very valuable and needed contribution to the FizzBuzzEnterpriseEdition project, but it currently seems to be failing our comprehensive test suite on Travis CI. I will be happy to merge it as soon as it is back to passing our tests.

@krasa
Copy link
Contributor Author

krasa commented May 26, 2015

thank you, interesting that this commit caused it: krasa@a7c7d84 :)
lets try rebuild....

@krasa krasa closed this May 26, 2015
@krasa krasa reopened this May 26, 2015
@codecov-io
Copy link

Current coverage is 85.43%

Merging #191 into master will change coverage by +1.44% by e433120

Coverage Diff

@@            master    #191   diff @@
======================================
  Files           59      60     +1
  Stmts          281     357    +76
  Branches        21      21       
  Methods          0       0       
======================================
+ Hit            236     305    +69
  Partial          7       7       
- Missed          38      45     +7

Powered by Codecov

@emiln
Copy link
Member

emiln commented May 26, 2015

I've requested a new build on Travis CI. It should start shortly.

emiln added a commit that referenced this pull request May 26, 2015
Best practices: use final wherever possible, constructor injection, this. access to fields
@emiln emiln merged commit 301d500 into EnterpriseQualityCoding:master May 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants