0

Everytime I input a new Item, the previous one added to the list is overwritten. I tried to run the debug and it seems that instead of creating a new item, the old one is just replaced with the new information. I've tried to look it up many times but I cannot find the problem.

There is no static variables and the instance is inside the loop for.

package application;

import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.Locale;
import java.util.Scanner;

import entities.Client;
import entities.Order;
import entities.OrderItem;
import entities.Product;
import entities.enums.OrderStatus;

public class Program {

    public static void main(String[] args) throws ParseException {
        
        
    Locale.setDefault(Locale.US);
    Scanner sc = new Scanner(System.in);
    SimpleDateFormat sdf = new SimpleDateFormat("dd/MM/yyyy");
    SimpleDateFormat sdf2 = new SimpleDateFormat("dd/MM/yyyy HH:mm:ss");
    
    
    System.out.println("Enter Client Data: ");
    System.out.println("Name:");
    String name = sc.nextLine();
    
    System.out.println("Email: ");
    String email = sc.nextLine();
    
    System.out.println("Birth Date (DD/MM/YYYY): ");
    Date date = sdf.parse(sc.next());
    Client client = new Client(name, email, date);
    
    System.out.println("\nEnter order data ");
    System.out.println("Status: ");
    OrderStatus status = OrderStatus.valueOf(sc.next());
    
    
    System.out.println("How many items to this order?");
    Integer itemsQuantity = sc.nextInt();
    sc.nextLine();
    Double price = null;
    Integer quantity = null;
    String pname = null;
    Date dateS = null;
    Order order=null;
    
    for(int i=0; i<itemsQuantity; i++) {
        
        System.out.printf("Enter #%d item data: ", i+1);
        System.out.println("\nProduct Name: ");
        pname = sc.nextLine();
        System.out.println("Product Price: ");
        price = sc.nextDouble();
        System.out.println("Quantity: ");
        quantity = sc.nextInt();
        if(i>=0) {
            sc.nextLine();
        }
        dateS = new Date(System.currentTimeMillis());
        Product product = new Product(pname, price);
        OrderItem orderItem = new OrderItem(quantity, price, product);
        order = new Order(dateS, status, client);
        order.addItem(orderItem);
    }

That is the class Order

package entities;

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

import entities.enums.OrderStatus;

public class Order {

    private Date moment;
    private OrderStatus status;

    private Client client;
    private List<OrderItem> items = new ArrayList<OrderItem>();

    public Order() {
    }

    public Order(Date moment, OrderStatus status, Client client) {
        this.moment = moment;
        this.status = status;
        this.client = client;
    }

    public Date getMoment() {
        return moment;
    }

    public void setMoment(Date moment) {
        this.moment = moment;
    }

    public Client getClient() {
        return client;
    }

    public void setClient(Client client) {
        this.client = client;
    }

    public OrderStatus getStatus() {
        return status;
    }

    public void setStatus(OrderStatus status) {
        this.status = status;
    }
    
    public List<OrderItem> getItems() {
        return items;
    }

    public void setItems(List<OrderItem> items) {
        this.items = items;
    }

    public void addItem(OrderItem item) {
        items.add(item);
    }

    public void removeItem(OrderItem item) {
        items.remove(item);
    }

    public Double total() {
        Double totalp = null;

        for (OrderItem i : items) {
            totalp =+ i.subtotal();

        }
        return totalp;
    }
    
    public void print() {
        for (OrderItem x : items) {
            System.out.println(x.getQuantity() +"   "+ x.getPrice() + "   "+ x.getProduct().getName());
        }
    }
    
}
  • 1
    "`order = new Order(dateS, status, client);`" you replace the value of `order` with a new one (empty list), and add one thing to it. Next iteration, you replace it again with a new one (empty list), and add one thing to it. The only outcomes here (barring exceptions) are that `order` is null, or it's a non-null value with one thing in it. – Andy Turner Sep 02 '21 at 15:37
  • 2
    And *please* don't use `Date` and `SimpleDateFormat` anymore. They're obsolete and [troublesome](https://stackoverflow.com/questions/1969442/whats-wrong-with-java-date-time-api). Use classes from the `java.time` package. For storing birthdate you probably want to use `LocalDate`, and for the `moment` of your `Order` probably an `Instant`. – MC Emperor Sep 02 '21 at 16:06
  • thanks for the answer @MCEmperor, I'll implement them:) – Game changer Sep 02 '21 at 16:20

1 Answers1

2

It is not overwritten the list in the Order. You are creating every time a new Order. So every time the new Order has an empty list of items and you add to it a single item having an order with a single item.

You need to move the code

order = new Order(dateS, status, client);

outside the for loop. You need also to put outside the for loop

dateS = new Date(System.currentTimeMillis());

but I suggest to remove the dateS at all and replace

order = new Order(dateS, status, client);

with

// It is not necessary to use the format new Date(System.currentTimeMillis())
// Because the default implementation of new Date take the 
// current time to init the date
order = new Order(new Date(), status, client);
Davide Lorenzo MARINO
  • 26,420
  • 4
  • 39
  • 56
  • `dateS` is assigned inside the loop body, so something different would need to happen there. – Andy Turner Sep 02 '21 at 15:40
  • @AndyTurner is not used (at least in the code posted here, so I suggested to remove it at all and replace with new Date() directly in the new Order parameters – Davide Lorenzo MARINO Sep 02 '21 at 15:43
  • @AndyTurner you are correct... but the intent was clear in my answer were dateS was replaced by new Date() directly in order = new Order(new Date(), status, client); removing the only place where dateS was used – Davide Lorenzo MARINO Sep 02 '21 at 15:46
  • Thank you so much for your answers, they fixed my problem :D and, the dateS would be used at the end, to show the order moment, that is why I used it inside the loop. @AndyTurner – Game changer Sep 02 '21 at 16:11
  • @ClébsonSouza And if you put the order outside the for loop you don't need it at the end... That's why it can be removed at all and replaced with new Date() – Davide Lorenzo MARINO Sep 02 '21 at 16:19