Problem
I’m trying to figure out some way to improve the code below. My main problem is the method getPrice
.
Some advices:
- In the
else
clause, I need to query the information in the database (using EJB injection). - The
ProductPrice
andMarketPrice
are very different JPA entities. So, I can’t refactor them to use inheritance/composition (per example).
The code is weird, but I can’t figure out a way to really improve.
import java.util.*;
import java.math.*;
import java.lang.*;
import java.io.*;
class Ideone
{
public static void main (String[] args) throws java.lang.Exception
{
PriceService priceService = new PriceService();
ProductPrice productPrice = new ProductPrice();
ProductMarketPrice productMarketPrice = priceService.getPrice(productPrice, 1L);
System.out.println(productMarketPrice.getAdvice());
productMarketPrice = priceService.getPrice(null, 1L);
System.out.println(productMarketPrice.getAdvice());
}
}
class PriceService {
//@EJB
MarketPriceService marketPriceService = new MarketPriceService();
public ProductMarketPrice getPrice(ProductPrice productPrice, Long productId) {
BigDecimal price = BigDecimal.ZERO;
Date date = null;
boolean isFromProduct = false;
if (productPrice != null) {
price = productPrice.getPrice();
date = productPrice.getRegisterDate();
isFromProduct = true; // i need to know if the values are from 'one' or 'x'
} else {
MarketPrice marketPrice = this.marketPriceService.findTheMarketPriceForProduct(productId);
price = marketPrice.getPrice();
date = marketPrice.getDate();
}
return new ProductMarketPrice(price, date, isFromProduct);
}
}
class ProductPrice {
public BigDecimal getPrice() {
return new BigDecimal("1.00");
}
public Date getRegisterDate() {
return new Date();
}
}
class ProductMarketPrice {
private BigDecimal price;
private Date date;
private boolean isFromProduct;
public ProductMarketPrice(BigDecimal price, Date date, boolean isFromProduct){
this.price = price;
this.date = date;
this.isFromProduct = isFromProduct;
}
public String getAdvice() {
String answer = this.isFromProduct ? "yes" : "no";
return "Hi, user! This price and date is from our Product? " + answer + ". So, be sure about that when you do your stuff.";
}
}
class MarketPriceService {
public MarketPrice findTheMarketPriceForProduct(Long productId) {
return new MarketPrice();
}
}
class MarketPrice {
public BigDecimal getPrice() {
return new BigDecimal("2.00");
}
public Date getDate() {
return new Date();
}
}
You can find the working code here.
Thanks!
Solution
isOne = true; // i need to know if the values are from 'one' or 'x'
The question is: why do you need to know that later?
My guess is because you later make a decision on that boolean value.
My suggestion is to create a ResultOne
class and a ResultX
class where both extend the Result
class.
The class Result
gets an additional (abstract) method which is implemented differently in ResultOne
and ResultX
.
from the comments:
Hi! The final user needs to know that the value is from EntityX, so this information is used to show as a advice to him.
abstract class Result {
// property and getter for value and date
abstract public String getResultTypeMessage();
}
class ResultOne extends Result{
public String getResultTypeMessage(){
return "this is result One";
}
}
class ResultX extends Result{
public String getResultTypeMessage(){
return "this is result X";
}
}
private Result getSomething(EntityOne entityOne, Long someId) {
Result result;
if (entityOne != null) {
result = new ResultOne(entityOne.getOneValue(),entityOne.getOneDate());
} else {
EntityX entityX = this.someEjb.findInDatabase(someId);
result = new ResultX(entityX.getOneValue(),entityX.getOneDate());
}
return result;
}
in your UI:
System.out.println("Type:"+ result.getResultTypeMessage());
System.out.println("Value:"+ result.getValue());
System.out.println("Date:"+ result.getDate());