-2

I am creating a program for uni that is about registering cars. I have an object class for the cars and an array list to keep them all in however when the addCar I made gets used and I then try to print it it will print the last one I entered for how ever many cars there should be

I have tried lifting the code straight into the main body and its the same and I've tried various different small changes like the way I'm comparing etc.

Here is the AddCar function that I keep in the Car class file

public static ArrayList<Car> addCar(ArrayList<String> Makes, Integer CarCounter, ArrayList<Car> Cars) {
    Integer Select;
    boolean Equal = false;

    for (int i = 0; i < Makes.size(); i++) {
        System.out.println(i + ": " + Makes.get(i));
    }

    Scanner choice = new Scanner(System.in);

    {
        System.out.println("Enter Numeric Choice: ");

        while (!choice.hasNextInt()) {
            System.out.println("Error Please Enter Numeric Choice Again: ");
            choice.next();
        }

        Select = choice.nextInt();
    }

    if (Select >= Makes.size()) { 
        System.out.println("No records exist (Number entered too large)");
    } else {
        for (int i = 0; i < Makes.size(); i++) {
            if (Equal = Makes.get(i).equals(Makes.get(Select))) {
                break;
            }
        }

        if (Equal == true) {
            Car newCar;
            Make = Makes.get(Select);
            Reg = Input.getString("What is the registration: ");
            Model = Input.getString("What is the Model: ");
            Colour = Input.getString("What is the Colour: ");
            newCar = new Car();
            newCar.setMake(Make);
            newCar.setReg(Reg);
            newCar.setModel(Model);
            newCar.setColour(Colour);
            Cars.add(CarCounter, newCar);
        } else {
            System.out.println("Make is unavailable Please Try Again");
        }       
    }

    return Cars;
}

Here is the line that declares the arraylist in the main body of the main file

ArrayList<Car> Cars = new ArrayList<Car>();

Heres the line that calls the function for one of the cases in my switch case menu

Cars = Car.addCar(Makes, CarCounter, Cars);

If for example I set the first car as fiat,L4QWS,Punto,Silver and the second car as Ferrari,4RE33,LaFerrari,Red, then it should be:

Car 1:
Make: fiat
Registration: L4QWS
Model: Punto
Colour: Silver

Car 2:
Make: Ferrari
Registration: 4RE33
Model: LaFerrari
Colour: Red

However, is actually comes out as:

Car 1:
Make: Ferrari
Registration: 4RE33
Model: LaFerrari
Colour: Red

Car 2:
Make: Ferrari
Registration: 4RE33
Model: LaFerrari
Colour: Red
  • Your problem is probably in the Car class. Perhaps all the member data of Car are static? –  May 02 '19 at 23:33
  • @another-Dave Yeah I'm not really sure of the difference between static and non static I have tried to change it but then it becomes a domino effect of removing static from everything until I get to where I'm calling the method in main and it doesn't like it – Kyle Halliday May 02 '19 at 23:36
  • static means shared between all object instances; so all your Cars will look the same. Is that indeed the problem? Maybe I should elevate my guess to an Answer. –  May 02 '19 at 23:38
  • Static values are shared among all instances of the class as they are instance independent. So remove the static declarations. – WJS May 02 '19 at 23:38
  • Ah yes that indeed sounds like the problem is there anywhere static should be a lot of the functions and classes are static swell – Kyle Halliday May 02 '19 at 23:42
  • @another-dave so once te static is removed from the fields and class on the main it produces an error as a static reference to the non static method cannot be made – Kyle Halliday May 02 '19 at 23:46
  • For this, pretty much *nothing* should be static except the ```main``` method in one class, and the first thing it should probably do is to create an instance of its class. –  May 03 '19 at 01:50

2 Answers2

0

It could be that CarCounter is never incremented and therefore when calling Cars.add(CarCounter, newCar) you always overwrite the first (and only) item in the list. You don't need CarCounter at all as it is redundant to Cars.size(). For adding an item to the list, simply use Cars.add(newCar) which will definitely add it to the end of the list. And speaking of Integer arguments: You are not calling the method by reference just by using Integer instead of int, and you should prefer the primitive type int for performance reasons. There are a few rare cases where you really want to pass an Integer object. And if you really want to pass the integer by reference, either enclose it into an array or use some wrapper class like AtomicInteger.

RedXIII
  • 781
  • 7
  • 6
  • Yeah no car counter definitely increments this is something I tested, when it prints form a loop the cars it prints the number that there should be correctly so for example it will print three cars if I tried to input one and 2 if I tried to input 2 just they will all become the same as the most recently entered one – Kyle Halliday May 02 '19 at 23:51
0

The problem is that the data members of the Car class are defined as static. This means that there is only one copy of such data members, shared between all instances of the Car class.

So, when you create a new Car and set its values, they show up through all Cars.

Generally speaking, static data should be the exception rather than the rule. The point about creating separate instances of the class is that they should really be separate.

  • Think this is it however the line from the question where I call the addCar method is now an error cause it is making a static reference to a non static method and I am unclear as to how to make this reference non static – Kyle Halliday May 02 '19 at 23:54
  • Step 1: have nothing static except ```main```, which has to be static. Step 2, assuming the class containing ```main``` is called ```CarRegister```, have ```main``` immediately and only execute ```CarRegister foo = new CarRgister();``` and then ```foo.someFunction();```. Step 3, Now you're executing in an instance of a CarRegister. Everything else should refer to an instance. For example, your collection of cars will be a member of the CarRegister instance. –  May 03 '19 at 23:22