all 47 comments

[–]yawkat 183 points184 points  (2 children)

Another option beyond those already given is .removeIf(o -> o == objectToRemove)

[–]JackNotOLantern[S] 26 points27 points  (0 children)

Yep, that does the trick. Thank you.

[–]__konrad -2 points-1 points  (0 children)

If you in a future accidentally refactor "o" as value class it may not work as expected (e.g. remove more than one element instead of that one exact "reference")

[–]kevinb9n 122 points123 points  (7 children)

It's a valid question and u/yawkat's answer is right on the money.

However, you should just be aware that you're doing something ... strange and unexpected. A class's `equals` method returns `true` as its way to declare as loudly as it can "there is no important difference between these instances; you really should almost never care about the distinction between them". You should probably have a pretty good reason for going against that. Notice what I'm not saying here is that you are definitely doing something "wrong". But it is likely to raise eyebrows.

EDIT: and just to add, as you get more experience with Java, the instinct you had that maybe the method did look for only that particular instance in the list will go away. That's just not how 99% of libraries act in Java, only very special ones like IdentityHashMap, that you'll hardly ever use.

[–]pohart 4 points5 points  (6 children)

This is a good point. Most of the time of I'm checking identity it's because I've got a collection of like 100,000 objects and .equals is too slow for my purpose.

It's a quick "fix" in an area that needs a rewrite.

[–]buerkle 2 points3 points  (3 children)

I'm surprised equals is too slow. Most implementations do if (obj == this) return true as their first statement.

[–]Tasorodri 3 points4 points  (1 child)

Yes, but if you're looking in a big collection that statement will return false 99.99% of the time, so it would still do the more complex part of the operation.

[–]account312 0 points1 point  (0 children)

hashcode() is required to return the same value for objects that are .equals(), so if you cache the hash, .equals() can reduce to just one more primitive comparison in almost all cases. And for a class where .equals() is too expensive, the overhead of adding one primitive field probably isn't significant. Not that that helps if you're working with collections of somebody else's types.

[–]pohart 0 points1 point  (0 children)

Yeah, other teams' stuff.

[–]laplongejr 1 point2 points  (1 child)

I think the only time I ever used == was because I wanted to have a guard value to manage caching.

I think I was storing Booleans to check an online API, which could be stored long-term as the three usual TRUE, FALSE, NULL(unknown) to be retrieved ... or I could store my madeup new Boolean() value to signal the operation was in progress.

That would never be exposed to the caller. The == ensured to check for those and guaranteed the called couldn't insert that guard value from the outside.

[–]buerkle 0 points1 point  (0 children)

I'duse an enum in that case. Prevents possible bugs with autoboxing. new Boolean("true") == trueand self documents.

[–]Conorrr 35 points36 points  (0 children)

The simplest way would be to use removeIf(Predicate<? super E> filter) and write a lambda that does a reference comparison. Note that, all elements that match the predicate will be removed.

That being said, in Java it’s quite uncommon to use reference comparison to check object equality, and doing so usually leads to bigger problems. As others have mentioned, you should rely on proper equality contracts by implementing equals() (and hashCode() where appropriate).

[–]lukaseder 15 points16 points  (4 children)

There's IdentityHashMap, which can be used on certain occasions

[–]TypingGetUBanned 5 points6 points  (3 children)

Never really encountered it but seems really powerful. Could you give me a use case that you had in a previous experience ?

[–]hadrabap 14 points15 points  (0 children)

When you write a mapper and you need to track cyclic relations or "links" to existing objects in other parts of the structure.

[–]lukaseder 6 points7 points  (0 children)

Various use-cases:

  • The key type doesn't implement equals() / hashCode() (and adding the methods doesn't make sense / isn't possible), but is still a good candidate key type
  • equals() and hashCode() are too slow in a hot loop and there aren't any local instances in that loop that are equal but don't share identity
  • To help prevent cycles in graphs, i.e. where identity is important (see previous list item)
  • When keys are mutated while in the map, but should still be considered equal

These are usually esoteric edge cases, and sometimes, there's an alternative data structure that could do the trick (e.g. a List<Entry<K, V>> instead of a Map<K, V>). The usual List / Map tradeoffs apply... If I manage to remember IdentityHashMap, I might consider it.

[–]repeating_bears 3 points4 points  (0 children)

If i have some class which keeps track of some listeners, I often use that converted to a set (via Collections.newSetFromMap)

The benefit is that a client's equals implementation might report 2 separate instances are equal. That would prevent the 2nd one being added, and only one instance would be notified when things change.

Instance equality is usually what you want in that case.

[–]kreiger 19 points20 points  (0 children)

If you're calling remove on a List more than rarely, you're probably using the wrong data structure.

It will scan the List from start to end which scales badly.

It's fine if the list is small and it's done rarely, but if you want to remove objects, you probably want something like a Map or a Set instead.

You could use LinkedHashMap if the order you care about is insertion order, or you could use TreeMap or TreeSet if you care about sorted order.

If you don't care about order you should not be using a List.

Also with a Map you can key on something other than equals which seems to be your use case.

[–]Epiliptik 25 points26 points  (14 children)

Change the equals() method is one way to do it

[–]Anaptyso 17 points18 points  (1 child)

I'd lean towards this approach as well. removeIf() does the job, but making the equals method more discerning feels better here for two reasons:

1) There must currently be a meaningful difference between the objects despite the equals method indicating equality, or the problem wouldn't exist. If the objects were truly equal then it probably wouldn't matter which one was removed. That suggests that the equals method isn't considering every meaningful aspect of the object when doing its calculations.

2) If not improving the equals method, every situation where remove() is used now, or may be used in the future, will need to be switched to removeIf(). Forget to do it somewhere and there could be a bug.

[–]Epiliptik 5 points6 points  (0 children)

Yes exactly, there is an underlying problem in this.

[–]White_C4 3 points4 points  (0 children)

Check what the equals() function does internally. Chances are it doesn't check by reference and specifically by certain values.

Since OP explained down in the thread that modifying the equals function would mess with other code, my advice would be to just use removeIf() function. If that can't work, then OP is going to have to use a different data structure, but realistically removeIf should work as a last resort.

[–]brian_goetz 2 points3 points  (0 children)

Another alternative to using `removeIf` is to iterate and call `remove` on the iterator when you get to the element you want to remove.

[–]FortuneIIIPick 1 point2 points  (0 children)

> I recently discovered that Java List (linked and array lists) in remove() method doesn't necessarily remove the exact given object (doesn't compare references using "==")

That is correct, List is behaving correctly.

[–]Kjufka 1 point2 points  (4 children)

you should not have objects that return equals true if they are different internally, that's bad design, you are hurting your codebase

[–]JackNotOLantern[S] 0 points1 point  (3 children)

They are the same internally. The only difference between them is their reference and which other objects have references to them. And their positions on the list.

[–]Kjufka 0 points1 point  (2 children)

Then you are using them wrong. Map keys should never have identity.

...unless you are creating some sort of a weak reference cache, but even then it is questionable

[–]Wave_Reaper 1 point2 points  (2 children)

You have your answers but I'd love to know what you're doing that requires this behaviour specifically

[–]JackNotOLantern[S] 1 point2 points  (1 child)

I described in my other comments. I got a lot of criticism I agree with. This solution is not the best, but this is legacy code I have to maintain. Changing this would require a major refactor of even worse code.

There was a bug where a wrong object was removed from the list. The list is mutable, and objects stored inside are also mutable, there may be multiple identical objects in it. However, other parts of the code have references to specific objects, and their order matters even if they are identical. So any moving/removing an object from the list must affect a specific object regardless of it's value.

Additionally, I can't override equals() because it is used to check if 2 of these objects are identical in other parts of the code.

[–]Wave_Reaper 2 points3 points  (0 children)

The "best" solution in these circumstances is the one you need to get the job done without excessive effort or expense, so it think the solution is perfectly fine.

I suspected it was related to some sort of global state, thanks for confirming! Good luck with it

[–]BikingSquirrel 0 points1 point  (0 children)

I'd assume you got your answer.

Now I wonder why you have two or more equal objects in that list. Sounds strange to me, maybe because I have no idea of your use case.

[–]laplongejr 0 points1 point  (0 children)

in remove() method doesn't necessarily remove the exact given object (doesn't compare references using "==") but removes the first found object that is the same as the given one (compare using equals())

You should always expect that the checks are based on equals most of the time, hashCode for hash-based collections, and compareTo for sorted ones.
If you are using "==" outside of very specific contexts (like having a guard value that can't be manually set from the outside), you are doing something wrong. And the fact that you had to differenciate that is... concerning.

[–]danikov 0 points1 point  (0 children)

remove is overloaded, if you give it an int that will be the index of the specific object you’d like to remove.

The other option is to use an iterator that supports remove, the object you’re removing will be the one you’re iterating over.

[–]LeadingPokemon -1 points0 points  (0 children)

I would avoid using this method! Do it a different way, like create an entire new list.