I have some concerns on my refactoring for the data layer in my current Project. Just for a small info, I am currently building a CRM as "training JEE application". But enough of talk, let's talk code!
IService.java: (7 lines, 160 bytes)
public interface IService<T> {
public void update(T update);
public void delete(long id);
public T getById(long id);
}
IListService.java: (8 lines, 169 bytes)
public interface IListService<T> extends IService<T> {
public void add(T add);
public List<T> getAll();
}
IDependentListService.java: (8 lines, 225 bytes)
public interface IDependentListService<T> extends IService<T> {
void add(T entity, long masterRecordId);
List<T> getAllByMasterRecordId(long masterRecordId);
}
IAddressService.java: (7 lines, 199 bytes)
public interface IAddressService extends IDependentListService<Address>{
public void promoteToMainAddress(long addressId);
}
IContactPersonService.java: (7 lines, 167 bytes)
public interface IContactPersonService extends IDependentListService<ContactPerson>{
}
IContractService.java: (9 lines, 234 bytes)
public interface IContractService extends IDependentListService<Contract> {
public List<Contract> getAllContractsByLoggedInUser();
}
ICustomerService.java: (9 lines, 220 bytes)
public interface ICustomerService extends IListService<Customer> {
}
IProductService.java: (10 lines, 262 bytes)
public interface IProductService extends IListService<Product> {
public List<Product> getAllCurrentProducts();
public List<Product> getAllArchivedProducts();
}
IProjectService.java: (9 lines, 228 bytes)
public interface IProjectService extends IDependentListService<Project>{
public List<Project> getAllProjectsByLoggedInUser();
}
IUserService.java: (7 lines, 129 bytes)
public interface IUserService extends IListService<User>{
}
Update (Code):
The code for the IUserService changed a bit, after reviewing the implementation of the CustomerService:
IUserService.java:
public interface IUserService extends IListService<User>{
User getLoggedInUser();
}
Questions
- Does this code follow good practices, especially concerning inheritance?
- Naming. Should I really prepend I before the Interfaces? (Implementations are named without
I
). Are the names clear enough? - Everything else you see as problematic ;)
Update (Questions):
I am currently in the process of writing Javadoc for this code after refactoring it. Currently I use the interfaces as a Platform to place the Javadoc. In the implementation I would then just @see
to the Interface Javadoc. Is it okay to do this, should I instead write out the Javadoc for the Implementations too and just @see
to other implementations and the interfaces?
-
6\$\begingroup\$ +1 simply for using my question generator tool :) \$\endgroup\$Simon Forsberg– Simon Forsberg2014年04月03日 15:09:28 +00:00Commented Apr 3, 2014 at 15:09
-
\$\begingroup\$ This question is currently referenced on Meta \$\endgroup\$rolfl– rolfl2014年08月19日 00:34:07 +00:00Commented Aug 19, 2014 at 0:34
1 Answer 1
This answer will reflect the naming convention at my work.
Naming Convention
Prefixing I
to all your interface seems superfluous, since we use a simple name like ContractService
. The implementation class would have Impl
suffixed to the name of the service like : ContractServiceImpl
. Normally you will use DI to inject the implementation where you need it, so you will only see the interface name.
Prefixing I
seems odd to me if I'm looking at the Java collections API where you have : List
, Map
, etc.
If you only use ContractService
as the name of the implementation, it could not be clear at first read that you are using an implementation directly and not the interface. Using Impl
as a suffix make it clear that it's the implementation, so if you see it in your code, it can show that something smelly is going on.
Naming of methods
public interface IProductService extends IListService<Product> { public List<Product> getAllCurrentProducts(); public List<Product> getAllArchivedProducts(); }
Those methods are very good! If you can keep clear names like those, using your services will be easy. I like the way you named all you method!
Delete
public void delete(long id);
You could return the object that has been deleted. It could be useful if you want to show the information of the item that has been deleted. The client could decide if he wants to use the object or not.
Edit: In fact, you should not return an object since you're just working with long id
and not the object directly. If the signature was public void delete(T delete)
this would have been a good advice, but with the current signature, you would have a to retrieve the object, delete it and then return the deleted object.