If/Else with multiple variable assignments. What pattern I can use to have a better code? [closed]

Posted on

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 and MarketPrice 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());

Leave a Reply

Your email address will not be published. Required fields are marked *