0

I am using the below approach,

public class Solution {
        public static void main(String[] args) {
            List<Car> cars = new ArrayList<Car>();
            cars.add(new Car(1,"A"));
            cars.add(new Car(2,"B"));
            cars.add(new Car(3,"C"));
            cars.add(new Car(4,"D"));
            cars.add(new Car(5,"E"));
            cars.add(new Car(6,"F"));
            if(cars.contains(new Car(4))){
                System.out.println(cars.get(cars.indexOf(new Car(4))));
            }
        }
    }

    class Car {
        int number;
        String name;
        Car(int number, String name){
            this.number = number;
            this.name = name;
        }
        Car(int number){
            this.number = number;
        }
        @Override
        public boolean equals(Object obj) {
            Car other = null;
            if(obj instanceof Car){
                other = (Car) obj;
            }
            return this.number == other.number;
        }
        @Override
        public int hashCode() {
            return number;
        }
        @Override
        public String toString() {
            return number + " " +name;
        }
    }

Can anybody please verify and let me know if this approach

cars.get(cars.indexOf(new Car(4)))

Please ignore the Car object I am creating here, That is only for understanding. In my case I need object with only one property to identify the object.

is correct, or if there is any better approach.

Thanks

Prakash
  • 13
  • 2
  • 9
  • You could use a `HashMap` instead. If you have many Cars it will be way faster than using `contains` on a `List`. (Also you get rid of the awkward constructor.) – froderik Feb 27 '17 at 07:54

5 Answers5

0

No, it's very confusing.

You're "hijacking" the equals method, which should really say if two cars are equal (and thus have the same number and the same name), just to be able to find back your car in the list using indexOf(). You're also create "invalid" Car objects (because a Car should always have a name) just to implement your specific use-case.

Just iterate the list to find the one which has the desired number:

Optional<Car> car = cars.stream().filter(car -> car.getNumber() == 4).findAny();

Or use a Map<Integer, Car> to be faster, if you often have to find cars by their number.

Also, every time you override equals, you should also override hashCode and respect their mutual contract (i.e. two equal objects MUST have the same hashCode).

JB Nizet
  • 678,734
  • 91
  • 1,224
  • 1,255
  • Yes I accept it is confusing, but where I want to implement it there this object is dependent on only single property where rest I can ignore and yes I am overriding Hashcode with that particular property so that it will not break the "contract". Can it make it acceptable? Thanks JB! – Prakash Feb 27 '17 at 08:38
  • No, not by my standards. – JB Nizet Feb 27 '17 at 09:22
0

You can get the location/position of it by running a loop and look for your desired number like this:

for (int i = 0; i < cars.size(); i++) {       
    if(cars.get(i).getNumber == 4){
            System.out.println(i); //this will print the position of Car in a list when number == 4
    }
}
Faraz
  • 6,025
  • 5
  • 31
  • 88
0

Your approach is correct to get an object by index's. But the complexity of code still O(n). But in this case I recommend your consider use HashMap

dphung duy
  • 19
  • 5
0

This is an unusual approach because by writing an equals() that ignores name you've made it so that two cars most people would expect to be unequal are seen to be equal.

That is:

  new Car(2,"Jaguar").equals(new Car(2,"Ford"))

... returnstrue, which is pretty irregular.

As it happens, in your small program it works -- but making equal() not work properly will cause all kinds of problems in a larger program.

You could also look at Maps:

Map<Integer,Car> map = new HashMap<>;
map.put(1,new Car("Jaguar"));
Car carOne = map.get(1);

See How to search in a List of Java object for better ways of finding an object in a list.

Community
  • 1
  • 1
slim
  • 40,215
  • 13
  • 94
  • 127
  • Hi Slim what if I have only one property to identify this object. Thinks as name is not available at all. So can it be a acceptable solution. Thanks! – Prakash Feb 27 '17 at 10:44
  • Then search for that property (see linked question), or use that property as the key for a Map. Google for the concept of the "Primary key". – slim Feb 27 '17 at 11:03
  • Apologies, I screwed up the Map example (typing on a phone!) -- have fixed now. – slim Feb 27 '17 at 11:04
0

For the record: your implementation of

@Override
public boolean equals(Object obj) {
  Car other = null;
    if(obj instanceof Car){
      other = (Car) obj;
    }
  return this.number == other.number;
}

is wrong on many levels. It will directly throw a NullPointerException if you call new Car(3).equals("some string"); for example.

Hint: your instanceof check may very well return false; and then you should not try to go for other.number!

GhostCat
  • 137,827
  • 25
  • 176
  • 248
  • Yes you are correct, this is for understanding only. My question is on how I am getting the object is correct way or not. Thanks! – Prakash Feb 27 '17 at 10:47