days
0
-28
-4
hours
-1
0
minutes
-5
-3
seconds
-3
-6
search
Untangling bad code

Pitfalls in designing the application’s business layer

Veselin Davidov
code
© Shutterstock /BEST-BACKGROUNDS

Even the simplest software can sometimes tangle up into spaghetti code and become a nightmare to navigate, especially in legacy systems. In this article, view some bad code from an application’s business layer and how to fix it with better design practices. Take care of nightmare code before it becomes too much to handle with these small improvements.

When we start writing software, we always want to have a good design. We read books, apply the best practices and often, in the end, we finish with a mess. In my experience in a custom software development company, I have to deal with such code on a daily basis, especially when working on some legacy systems.

There are different reasons for that, and I will try to cover some of them in a series of articles trying to look at them in a practical way. In my first example, I will show why simple software can evolve into a nightmare and I will suggest a slight improvement. I will focus only on the service layer that handles the business logic.

SEE ALSO: Load balancing: Round robin may not be the right choice

First, some bad code that we see (and write) so often in reality

Let’s start with a simple storage application. We have products resource with service, repository and we can do CRUD operations which is what we think we need. Our product service looks something like:

public class ProductService {
	public String create(Product product) {
		return productRepository.create(product);
	}
	public String update(Product product) {
		return productRepository.update(product);
	}
	public Product get(String productId) {
		return productRepository.get(productId);
	}
	public void delete(Product product) {
		productRepository.delete(product);
	}
}

There will be some other stuff like DTO to Entity mapping, controllers etc. But as I said, we will consider them written and ready for simplicity. Our Product entity is simple java bean, our repository saves in the correct DB table. Then we get another requirement that we will also create an online store and we need a way to place orders. So, we add a quick order service to cover our still simple requirements:

 public class OrderService {
	public String saveOrder(Order order) {
		return orderRepository.save(order);
	}
}

It’s simple, readable and works! Then a new requirement comes to update the products in stock when an order is placed. We do it like:

public class OrderService {
	public String saveOrder(Order order) {
		Product product=productService.get(order.getProductId());
		product.setAvailableQuantity(product.getAvailableQuantity()-order.getQuantity());
		productService.update(product);
		
		return orderRepository.save(order);
	}
}

You can probably see where I am going but this is still readable and works fine. After that, we get three more requirements. 1/ we need to call the shipping service to ship that product to an address 2/ throw an error if there is not enough stock to fulfill the order 3/ if the product’s available quantity goes under some minimum to restock. And there it goes:

public class OrderService {
	public String saveOrder(Order order) {
		Product product=productService.get(order.getProductId());
		
		//The order service works more like a product service in the following liness
		if(product.getAvailableQuantity()<order.getQuantity()){
			throw new ProductNotAvailableException();
		}
		product.setAvailableQuantity(product.getAvailableQuantity()-order.getQuantity());
		productService.update(product);
		if(product.getAvailableQuantity()<Product.MINIMUM_STOCK_QUANTITY){
			productService.restock(product);
		}
		
		//It also needs to know how shipments are created
		Shipment shipment=new Shipment(product, order.getQuantity(), order.getAddressTo());
		shipmentService.save(shipment);
		
		return orderRepository.save(order);
	}
}

I know it might be an extreme example, but I am sure we have seen similar code in our projects. There are multiple problems with that – shared responsibilities, messing with other domains logic and infrastructure etc. If it was a real-life store, then the guy that takes the order would be like the general manager – taking care of everything from taking the actual order to stock maintenance and delivery.

Now to a slightly better version

Let’s try to handle the same scenario in a different way. I will start with the order service. Why do we call our method saveOrder? Because we look at it as developers and not from a business perspective. Our developers’ minds are often database driven (or REST driven) and we see our software as a series of CRUD operations. Usually, when we look at books for Domain-Driven Design there is a term mentioned Ubiquitous Language – common language between the developers and users. If we try to model the business in our code why not using the correct terms. We can change our initial code to:

public class OrderService {
	public String placeOrder(Order order) {
		return orderRepository.save(order);
	}
}

A small change but even that makes it more readable. It’s a business layer, not a DB layer – we place orders when we go to the store, we don’t save them. Then when the other requirements come instead of start coding them using our existing services with CRUD operations, we can try to recreate the business model. We ask the business guys and they tell us that when the order is placed the guy who took it calls the stock department and ask them if the product is available, then reserves it and calls the delivery guys with the reservation number and address so they can ship it. What stops us to do the same in our code?

public class OrderService {
	public String placeOrder(Order order) {
		String productReservationId=productService.requestProductReservation(order.getProductId, order.getQuantity());
		String shippingId=shipmentService.requestDelivery(productReservationId, order.getAddressTo());
		order.addShippingId(shippingId);
		return orderRepository.save(order);
	}
}

In my opinion, it looks much cleaner and represents the sequence of events that happen in the actual store. The order service doesn’t need to know how products work or how shipping works. It just uses the methods needed to do its job. We will need to modify the other services too:

public class ProductService {
	//Method used in Orders Service
	public String requestProductReservation(String productId, int quantity){
		Product product=productRepository.get(productId);
		product.reserve(quantity);
		productRepository.update(product);
		return createProductReservation(product, quantity);
	}
	
	private String createProductReservation(Product product, int quantity){
		ProductReservation reservation=new ProductReservation(product,quantity);
		reservation.setStatus(ReservationStatus.CREATED);
		return reservationRepository.save(reservation);
	}
	
	//Method used in Shipment Service
	public ProductReservation getProductsForDelivery(String reservationId){
		ProductReservation reservation=reservationRepository.getProductReservation(reservationId);
		reservation.getProduct.releaseReserved(reservation.getQuantity());
		if(reservation.getProduct().needRestock()){
			this.restock(product);
		}
		reservation.setStatus(ReservationStatus.PROCESSED);
		reservationRepository.update(reservation);
	}
}

The product service exposes two methods to be used from the other services but doesn’t know anything about their structure. It doesn’t care about orders, shipments etc. The logic when a product needs restocking and if a product has enough quantity is inside the actual product.

SEE ALSO: Python tutorial: Best practices and common mistakes to avoid

public class Product() {
	//Fields, getters, setters etc...
	
	public void reserve(int quantity){
		if(this.availableQuantity - this.reservedQuantity > quantity){
			this.reservedQuantity+=quantity;
		} else
			throw new ProductReservationException();
	}
	public releaseReserved(int requested){
		if(this.reservedQuantity>=requested){
			this.reservedQuantity-=requested;
			this.availableQuantity-=requested;
		} else 
			throw new ProductReservationException();
	}
	public boolean needsRestock(){
		return this.availableQuantity<MINIMUM_STOCK_QUANTITY;
	}
}

And the shipment service can be something like that:

public class ShipmentService {
	public String requestDelivery(String reservationId, Address address){
		ProductReservation reservation=productService.getProductForDelivery(reservationId);
		Shipment shipment=new Shipment(reservation, address);
		return shipmentRepository.save(shipment);
	}
}

I am not saying it is the best design but I think it is much cleaner. Each service takes to care for its own domain and knows as little as possible about the others. The actual entities are not just data holders also carry the logic related to them so the service doesn’t need to modify their internal state directly. And what’s most valuable in my opinion is that the code really represents how the business works.

Conclusion

If we don’t go into the situation from the first part of the article, we should try to take our time and understand our model properly. Even if new requirements come and we are pressured by time or if it will take more time to refactor, we shouldn’t be lazy. Mixing logic from different domains in services and entities seems maintainable at first but becomes spaghetti when the project grows bigger. Just as in our real-life store example – a small online store owner can take care of everything from taking orders, stocking, delivery, and finance. But when the store grows, he won’t be able to and it will become a mess.

Author
Veselin Davidov
Veselin Davidov is a dev-lead and full-stack developer at Dreamix, a custom software development company. He has experience in all spheres of software development and has extensive knowledge in designing, developing and delivery of large enterprise systems with Java, Spring and the surrounding technologies. He is also passionate about data and has led to the success of various Big Data and BI projects. You can find him on LinkedIn.

Leave a Reply

avatar
400