Best practices for coding
Fail fast
If a method receives an invalid argument (or has invalid global variables) and it is not buid to deal with that kind of state, throw an IllegalArgumentException (or an IllegalStateException). Do not just ignore it. Even allowing an NullPointerException to be thrown is fine.
Whatever the case, the application should not continue and misbehave without notification, lowering the user's trust in the application. Also, by failing fast, it's much easier to see the correct cause of a problem/misconfiguration and fix it.
For example:
public void solvingStarted(LocalSearchSolverScope localSearchSolverScope) {
if (completeTabuSize < 0) {
throw new IllegalArgumentException("The completeTabuSize (" + completeTabuSize
+ ") cannot be negative.");
}
...
}
Almost never return null
No results in the find method? Return an empty list (or collection or map), do not return null.
Something went wrong in a method? Don't return null, throw an exception (fail fast)!
Use final
Prevention of accidental assigning
High cohesion classes
Class has only one responsibility.
Inheritance
Make sure objects share common theme. http://en.wikipedia.org/wiki/Liskov_substitution_principle
Composition
In most cases composition can replace inheritance
Replace inheritance by delegation.
Short methods
Method should do only one thing.
Proper names for variables, functions and classes
They should describe to reader what it does or contains. Use behaviour while naming. http://en.wikipedia.org/wiki/Behavior_Driven_Development
Don't use abbreviations (ms for modeshape, jr for Jackrabbit,...), unless an average java programmer knows the abbreviation (HTTPS, URL, ...)
Exceptions
Throw (OR log). Never throw and log. http://today.java.net/pub/a/today/2006/04/06/exception-handling-antipatterns.html
Prefer throwing instead of logging, so it bubbles up the stack and the end-user can be notified. On the top of the execution stack there is (or should be) a generic exception handler, which 1) logs the exception AND 2) notifies the user. In a client-server architecture, every server side exception should be logged by both the generic server exception handler as the generic client exception handler (and the latter also notifies the user).
A good generic exception handler can also tell the user if it's their fault (wrong password, not unique name, required field empty, ...) or the programmers fault (NPE, IllegalStateException, ...) and show that differently to the end-user.
Design by contract
You can trust your own code. http://en.wikipedia.org/wiki/Design_by_contract
JavaDocs
Minimal JavaDocs
Don't bother javadoccing the obvious like "Gets score" for getScore(): normally the method name should be self-explaining enough.
Do, at least, a minimal of javadoccing of parameters and returns with "never null", "sometimes null" , "never negative", ...:
For example:
/**
* @param scoreString never null
* @return never null
*/
Score parseScore(String scoreString);
Document corner cases in the javadoc:
/*
* Returns a Score whose value is (this * multiplicand).
* When rounding is needed, it should be floored (as defined by {@link Math#floor(double)}.
Consider adding examples in the javadoc if and only if it's needed for clarity:
* Parsing "01-02-2003" returns the 2th of january of 2003.
Always document GWT Events
The person who creates a new Event class MUST document what is it for and under which circumstances it is fired. This way, another person could decide whether he can use an existing event for a new feature or if he needs to define a completely new one.
Ask
If you are not sure. Ask!
About MVP
Refactoring to MVP using with unit tests
1. I changed the class name for the class I wanted to MVP MyClass -> MyClassViewImpl
2. I created the MyClassPresenter class
3. I created an unit test class for MyClassPresenter called MyClassPresenterTest
4. I created a failing unit test for some feature/logic that is in MyClassViewImpl
5. I moved the logic needed for the unit test from MyClassViewImpl to MyClassPresenter
6. Refactored the code to look good
7. Repeated 3 to 6 until all the logic was out of the MyClassViewImpl
I know this is slow, but in the end you have a good test coverage and the unit test forces you to do the right thing.
Now if you follow the steps above, you notice that you have to:
* create the MyClassView interface
* pass the RPC service classes ( usually the model, but some classes like the one Jervis picked only use the service ) and the MyClassView interface in the constructor
Mocking
The problem with the view implementation and RPC services is that they use GWT.create(). When you do the unit test you will notice that they cause errors. This is why they are passed in, in the constructor using interfaces. This way you can also mock them in the unit test.
Mock the view interfaces using Mockito
* This way you can set up what you wan't that the methods return
* You can also verify that the methods where called and that they were called with the right values
Mock the RPC services using an actual Mock Class
* I feel this is a cleaner way to set up the services as they usually return list based on some parameters. It is possible to use mockito for this too
An example that covers these both:
Use Presenter interface to listen to events
So instead of adding the click listeners to the view I'm using onEvent() to listen to these events.
interface MyClass View{
interface Presenter{
onSave();
onClose();
}
void setPresenter(Presenter presenter);
}
This way I don't have to care what the actual source(s) of this event is and the method clearly states what it does.
Query and Command the View
For example in MyClassPresenter.onClose() I might have this:
public void onRemovePerspective(){
// Query the selected uuid
String selectedPerspectiveUuid = view.getSelectedPerspectiveUuid();
if (selectedPerspectiveUuid == null) {
// Command the view to show an error.
view.showNoSelectedPerspectiveError();
} else {
deletePerspective(selectedPerspectiveUuid);
}
private void deletePerspective(final String selectedPerspectiveUuid) {
configurationServiceAsync.remove(selectedPerspectiveUuid, new GenericCallback<Void>() {
public void onSuccess(Void result) {
// Remove the item from the view
view.removePerspective(selectedPerspectiveUuid);
}
});
}
}
Comments