11
\$\begingroup\$

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?

\$\endgroup\$
10
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.