1
\$\begingroup\$

I have an object (ShoppingCart) that has a list of CartItems, which contains the related Product and the quantity bought.

public class ShoppingCart extends BaseEntity {

    @OneToMany(mappedBy = "shoppingCart", cascade = CascadeType.ALL, orphanRemoval = true)
    private List<CartItem> cartItems = new ArrayList<>();

    @DateTimeFormat(pattern = "dd/MM/yyyy hh:MM:ss")
    private LocalDateTime dateTime;

    @Enumerated
    private PaymentMethod paymentMethod = PaymentMethod.CASH;
public class CartItem extends BaseEntity {

    @ManyToOne
    private Product product;

    @ManyToOne(fetch = FetchType.LAZY)
    @JoinColumn
    private ShoppingCart shoppingCart;
public class Product extends BaseEntity {

    @NotEmpty
    private String name;

    private double price;

    private int quantity;

I have a view that sums all shopping carts, and I don't know if I should store that sum inside ShoppingCart entity, or iterate to find the sum. Here's the code, right now I'm iterating.

@GetMapping("/")
    public String getProduct(@ModelAttribute("reportDto") ReportDto reportDto, Model model) {

        double total = 0, cash = 0, credit = 0, debit = 0;
        int quantity;
        List<ShoppingCart> results;
        if (reportDto.getBeginDate() == null || reportDto.getEndDate() == null) {
            results = shoppingCartService.findAll();
        } else {
            results = shoppingCartService.findByDateTimeBetween(reportDto.getBeginDate(), reportDto.getEndDate());
        }
        total = results.stream()
                .mapToDouble(shoppingCart -> shoppingCart.getCartItems().stream()
                        .mapToDouble(cartItem -> cartItem.getProduct().getPrice() * cartItem.getQuantity()).sum())
                .sum();
        quantity = results.stream().mapToInt(
                shoppingCart -> shoppingCart.getCartItems().stream().mapToInt(cartItem -> cartItem.getQuantity()).sum())
                .sum();

        cash = results.stream().filter(shoppingCart -> shoppingCart.getPaymentMethod().isCash())
                .mapToDouble(shoppingCart -> shoppingCart.getCartItems().stream()
                        .mapToDouble(cartItem -> cartItem.getProduct().getPrice() * cartItem.getQuantity()).sum())
                .sum();

        credit = results.stream().filter(shoppingCart -> shoppingCart.getPaymentMethod().isCredit())
                .mapToDouble(shoppingCart -> shoppingCart.getCartItems().stream()
                        .mapToDouble(cartItem -> cartItem.getProduct().getPrice() * cartItem.getQuantity()).sum())
                .sum();

        debit = results.stream().filter(shoppingCart -> shoppingCart.getPaymentMethod().isDebit())
                .mapToDouble(shoppingCart -> shoppingCart.getCartItems().stream()
                        .mapToDouble(cartItem -> cartItem.getProduct().getPrice() * cartItem.getQuantity()).sum())
                .sum();
        model.addAttribute("quantity", quantity);
        model.addAttribute("cash", cash);
        model.addAttribute("credit", credit);
        model.addAttribute("debit", debit);
        model.addAttribute("total", total);
        return "reports/reports";
    }

Also, any feedback regarding the code will be appreciated!

\$\endgroup\$
1
\$\begingroup\$

First of all i think making a view in your database which sums your needs will be better because generally sql engine sum operator's performance is way better than lopping values in programming languages.

Second if we go through your code: You have done mapping for all objects one by one. Java stream mapping is similar to for loops in compile time. So here what you make is similar to creating for loop foreach object which is bad for performance. To be performance wise you can create one for loop and generate your sum values in it.

Note: BigDecimal is the preferred way to store double money values in Java

\$\endgroup\$
  • \$\begingroup\$ I'll refactor and create separate methods to get the sum directly from the database (throught spring @repository, using @Query). I'll also change from double to BigDecimal. I understood the current loop problem, but since i'll change to database sum, I won't be able to sum it all in 1 loop, right? I'll have to do separate calls, for each sum. \$\endgroup\$ – fsakiyama Dec 20 '18 at 9:55
  • \$\begingroup\$ Hi @fsakiyama In database when you create your view try to put all sum operations to different columns means for instance => create view as select sum(cash) as cash, sum(quantity) as quantity and other columns from ... Then you can get all your results with only one database call. \$\endgroup\$ – Serhat Oz Dec 20 '18 at 10:53
  • \$\begingroup\$ Well, I'll still have to call the method X times (sum(cash), sum(quantity), etc), but at least it will be one method only! Thanks! \$\endgroup\$ – fsakiyama Dec 20 '18 at 17:55
0
\$\begingroup\$

Storing the sum in ShoppingCart or computing it is up to you. I will compute it each time and eventually "cache" it if there is a lot of items.

However your code will be more maintainable by enforcing encapuslation and moving all this computation code in the ShoppingCart itself. You can also continue with encapsulation by adding a getUnitPrice():double and getPrice() in your CartItem.

--

Note that double is not a good type for money.

\$\endgroup\$
  • \$\begingroup\$ I've decided to use get the sum directly from the database (From '@repository', using '@Query'). Also changing from double to BigDecimal. About encapsulation, I got your point and I kinda agree with you, but since I'm using Spring, I'm not sure about injecting the '@repository' inside the entity to do the sum. Maybe because I'm using the database sum approach, I should leave the logic to a service? \$\endgroup\$ – fsakiyama Dec 20 '18 at 10:02
  • \$\begingroup\$ If you want to place the sum in your @Entity you should not use SQL to compute it. At some time you should decide of your architectural patterns and choose one. Are you really sure that executing an SQL query to get the sum of something that is alrdeay loaded is the best approach ? \$\endgroup\$ – gervais.b Dec 20 '18 at 10:47
  • \$\begingroup\$ I decided the sql sum over iterating inside the entities. Thanks for pointing it out! \$\endgroup\$ – fsakiyama Dec 20 '18 at 17:54

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.