Doubt about the price calculation

Question

My doubt may seem simple to someone, but to me it’s kind of killing me because I’m trying to find the mistake and I’m not getting it.

At first, I know very little about object-oriented programming in Java, since most of the time in my course I only use C or C++. So I decided to take a course in Java to learn a little more, but apparently it’s very different from what I know how to do.

So I’m going to pass the question here and my code plus I’m not able to calculate the total price.

Create a class Pizza which has the method adicionaIngrediente() who receives a String with the ingredient to be added. This class must also have the method getPreco() which calculates as follows: 2 ingredients or less cost 15 real, 3 to 5 ingredients cost 20 real and more than 5 ingredients costs 23 real.

You need to account for the ingredients spent on all pizzas! Use a static variable in the class Pizza to store this type of information (tip: use the class HashMap to keep the ingredient as key and a Integer as value). Create static method contabilizaIngrediente() to be called within adicionaIngrediente() and make that record.

Create a new class called CarrinhoDeCompras which can receive class objects Pizza. It must have a method that returns the total value of all pizzas added. Cart cannot accept adding a pizza without ingredients.

Create a class Principal with the method main() which does the following:

• Creates 3 pizzas with different ingredients.

• Add these Pizzas in a CarrinhoDeCompra.

• Prints the total of CarrinhoDeCompra.

• Prints the amount of each ingredient used.

Then, just below I tried to make the calculation of the total price but it returns me the value as zero. I spent a few hours trying to find what could be the error that it has to return me only zero, then I decided to come here in stackoverflow where I have already solved many my doubts in C++.

import java.util.*;

public class Pizza {

static HashMap<String, Integer> ingrediente = new HashMap<String, Integer>();   
ArrayList<String> qntdadeingre;
static int totalingre=0;
static int total=0;
static int preco=0;

public Pizza()
{
    this.qntdadeingre = new ArrayList<String>();
}

public static void contabilizaIngrediente(String ingredientes, Integer qntidade)
{
    ingrediente.put(ingredientes,qntidade);
}

public void adicionaIngrediente(String ingredientes)
{
    this.qntdadeingre.add(ingredientes);
    if(Pizza.ingrediente.containsKey(ingredientes)) {
        Pizza.contabilizaIngrediente(ingredientes, Pizza.ingrediente.get(ingredientes) + 1);
    } else {
        Pizza.contabilizaIngrediente(ingredientes, 1);
    }
}

public int getPreco()
{
    if(totalingre<=2)
    {
        preco=15;
    }else 
    {
        if(totalingre <= 5)
        {
            preco=20;
        }else
        {
            preco=23;
        }
    }
    total +=preco;

return total;
}

public int contemingr()
{
    if(qntdadeingre.size() > 0)
        return 1;
    else
        return 0;
}

public HashMap<String, Integer> getIngrediente()
{
     return ingrediente;
}

static void printatotal()
{
    System.out.println("Preco Final: " + totalingre);

}
}

Answer 1

Let’s start by observing some things from your code:

1. Don’t forget the modifier private. Hardly the package visibility is what you want.

2. It is not good practice to abbreviate or eat letters of variable names, methods, classes, etc. So, qntdadeingre, totalingre and contemingr are not good names. It is also good idea you respect the language convention, which means that when variable and method names have more than one word, you will use uppercase letters in the name of subsequent words.

3. If you already have the list of ingredients (qntdadeingre), so you don’t need a variable to track the amount of ingredients (totalingre), just check the list size.

4. The method contabilizaIngredientes should not receive the amount of ingredients to be accounted for, only the ingredient to be accounted for. That method shall add one to the quantity of such ingredient. The complexity/responsibility of accounting for this ingredient is something that should only be in this method, but if it gets the amount from outside, then it wouldn’t be counting, it would just be putting in the HashMap. Moreover, this method should be the only one to use the HashMap, time that HashMap serves only the internal purposes of this method.

5. There is an object-oriented programming principle that tells you to rely on abstractions/interfaces and not implementations. When using type variables ArrayList and HashMap instead of List and Map, you are violating that principle.

6. When placing the modifier static in the field totalingre you ended up making all the pizzas have the same price! The price has to be calculated by the amount of ingredients in the pizza and not the total of pizzas.

7. Do not use static variables or instance variables for things that can be solved with local variables. This is the case for your variable preco.

8. There is no reason to count the total price of pizzas, since the statement does not ask for any of this. What he asks is the total of pizzas inside a shopping cart. Hence, the variable total is in the wrong class and it won’t compute what you want it to compute in it.

9. Names in the singular must refer to something that represents a single thing, while names in the plural must refer to collections of things. However, you violate that concept by doing ingrediente, qntdadeingre be collections and getIngrediente() be a method that returns a collection while ingredientes is the name of a single ingredient.

10. Do not use int to represent false or true with 0 or 1, use the type boolean with true or false. Remember that Java is not C or C++!

11. You’re still far from the point where you can learn the MVC pattern, but know that mixing visualization logic with modeling logic is not a good idea. The classes Pizza and CarrinhoDeCompras serve to model concepts of the domain of your system. Already instructions of type System.out.println are part of the visualization logic, ie to display information to the user. I know this is an advanced concept to be introduced to you now, but try not to mix things that serve to interact with the user within things that shape your domain. Your method printatotal() is a violation of this concept because it interacts with the user (displaying a message), but is within a domain class (Pizza).

Your code should look like this:

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class Pizza {
    private static final Map<String, Integer> quantidadesPorIngrediente = new HashMap<>();

    private final List<String> ingredientes;

    public Pizza() {
        ingredientes = new ArrayList<>();
    }

    private static void contabilizaIngrediente(String ingrediente) {
        quantidadesPorIngrediente.compute(ingrediente, (k, v) -> v == null ? 1 : v + 1);
    }

    public void adicionaIngrediente(String ingrediente) {
        ingredientes.add(ingrediente);
        contabilizaIngrediente(ingrediente);
    }

    public boolean temIngredientes() {
        return !ingredientes.isEmpty();
    }

    public int getPreco() {
        int quantidadeIngredientes = ingredientes.size();
        return quantidadeIngredientes <= 2 ? 15 : quantidadeIngredientes <= 5 ? 20 : 23;
    }

    public static Map<String, Integer> getQuantidadesPorIngrediente() {
        return quantidadesPorIngrediente;
    }
}

import java.util.ArrayList;
import java.util.List;

public class CarrinhoDeCompras {
    private final List<Pizza> pizzas;

    public CarrinhoDeCompras() {
        pizzas = new ArrayList<>();
    }

    public void adiciona(Pizza p) {
        if (!p.temIngredientes()) throw new IllegalArgumentException();
        pizzas.add(p);
    }

    public int getPreco() {
        int total = 0;
        for (Pizza p : pizzas) {
            total += p.getPreco();
        }
        return total;
    }
}

import java.util.Map;

public class Principal {
    public static void main(String[] args) {
        Pizza marguerita = new Pizza();
        marguerita.adicionaIngrediente("Queijo");
        marguerita.adicionaIngrediente("Tomate");
        marguerita.adicionaIngrediente("Orégano");

        Pizza portuguesa = new Pizza();
        portuguesa.adicionaIngrediente("Queijo");
        portuguesa.adicionaIngrediente("Presunto");
        portuguesa.adicionaIngrediente("Ovo");
        portuguesa.adicionaIngrediente("Calabresa");
        portuguesa.adicionaIngrediente("Cebola");
        portuguesa.adicionaIngrediente("Tomate");

        Pizza brigadeiro = new Pizza();
        brigadeiro.adicionaIngrediente("Brigadeiro derretido");

        CarrinhoDeCompras c1 = new CarrinhoDeCompras();
        c1.adiciona(marguerita);
        c1.adiciona(portuguesa);
        c1.adiciona(brigadeiro);
        System.out.println("Preço do carrinho: " + c1.getPreco());

        for (Map.Entry<String, Integer> entry : Pizza.getQuantidadesPorIngrediente().entrySet()) {
            System.out.println("O ingrediente " + entry.getKey() + " foi usado " + entry.getValue() + " vezes.");
        }
    }
}

Here is the output that is produced:

Preço do carrinho: 58
O ingrediente Queijo foi usado 2 vezes.
O ingrediente Cebola foi usado 1 vezes.
O ingrediente Tomate foi usado 2 vezes.
O ingrediente Ovo foi usado 1 vezes.
O ingrediente Orégano foi usado 1 vezes.
O ingrediente Calabresa foi usado 1 vezes.
O ingrediente Presunto foi usado 1 vezes.
O ingrediente Brigadeiro derretido foi usado 1 vezes.

See here working on ideone

Now, I will make a few comments on some of the problems with the wording of the exercise:

• Use a HashMap static is not a good practice of object-oriented programming. This HashMap is nothing more than a global variable, which is a bad programming practice even in the structured programming of C. Exercises that teach, suggest or stimulate bad programming practices are not good (unless they clarify this fact later). Ideally, this accounting of ingredients would have some context represented by some object, such as a Pizzaria. In well-written object-oriented programs, you will find few fields/attributes with the modifier static which are not numerical constants or strings.

• If the idea is to learn object-oriented programming, the ingredient should be represented by a class Ingrediente, and not by a single String.

• Exercise doesn’t tell you what happens if you try to create a pizza with repeated ingredients. The above implementation lets this happen and counts multiple times the repeated ingredient.

• The exercise says that pizzas without ingredients should be refused, but it does not say exactly which refusal mechanism should be used. In this case, I used an exception to signal the refusal. And although he asks to encode this refusal, he does not ask to exercise it in the main.

• Building pizzas without ingredients and then adding them is not one of the best programming practices, since it leads the programmer to the addiction of creating objects that are built incomplete to be filled later (and this vice is extremely common and widespread and difficult to combat). The ideal is that the constructor already returns the object ready to be used with all its fields properly defined. Look at this other question. In this case, the ideal would be that the builder of Pizza already received all ingredients to be used as constructor parameters.