- Dzielenie na warstwy wyszło już z użycia, kiedyś była to powszechna praktyka, teraz robi się pakiety per domena, czyli w twoim przypadku pakiet cart, order, customer zamiast model, service itp. Korzyści z tego, masz porządek, wszystko dotyczące klienta w jednym pakiecie, jak np. to wszystko dotyczące klienta się rozrośnie, możesz łatwo zrobić refaktor i nowy websevice wydzielić.
- nazwy pakietów z małej litery (to tylko taka konwencja programistów javy)
- Używaj dobrodziejstw javy 8+, np. stream().filter
Przykład zamiast:
private CartLineInfo findLineByCode(String code) {
for (CartLineInfo line : this.cartLines) {
if (line.getProductInfo().getCode().equals(code)) {
return line;
}
}
return null;
}
możesz napisać:
private Optional<CartLineInfo> findLineByCode(String code) {
return cartLines.stream()
.filter(line -> line.getProductInfo().getCode().equals(code))
.findFirst();
}
Nawiasem mówiąc optional też jest spoko zamiast nulla.
Jeszcze to get get jest słabe, to też można poprawić np. wprowadzając w klasie CartLineInfo metodę getProductCode i wtedy to wygląda tak:
private Optional<CartLineInfo> findLineByCode(String code) {
return cartLines.stream()
.filter(line -> line.getProductCode().equals(code))
.findFirst();
}
coraz lepiej co nie :)
-
Metody addProduct , updateProduict spodziewałabym się w osobnej klasie. Mieszasz dane z logiką biznesową.
-
Zamiast:
if (code != null && code.length() > 0)
możesz zapisać (to jest to samo, ale metoda już nazwana i wygląda czytelniej):
if (StringUtils.isNotEmpty(code))
- Masz (to jest ok):
public ProductForm() {
this.newProduct= true;
}
ale potem bez sensu ustawiasz newProduct, bo już jest true
productForm = new ProductForm();
productForm.setNewProduct(true);
- Zamiast:
public Product findProduct(String code) {
try {
String sql = "Select e from " + Product.class.getName() + " e Where e.code =:code ";
Session session = this.sessionFactory.getCurrentSession();
Query<Product> query = session.createQuery(sql, Product.class);
query.setParameter("code", code);
return (Product) query.getSingleResult();
} catch (NoResultException e) {
return null;
}
}
możesz użyć klasy repozytorium z arsenału gotowców springa:
public interface ProductRepository extends CrudRepository<Product, Long> {
List<Product> findByCode(String code);
}
To samo, a mniej kodu, większa czytelność.
-
to ciągłe sprawdzanie != null wygląda słabo
-
poczytaj o testach jednostkowych i Junit
-
wstrzykiwanie przez konstruktor
-
EntityManager lepsze od SessionFactory:
https://stackoverflow.com/questions/5640778/hibernate-sessionfactory-vs-entitymanagerfactory
itd. ale na dzisiaj starczy :)