Bug - Fixed Comparator for Concoction is buggy

[Veracity]

Apparently. I was moving 1700 items from inventory into my closet, which forced a Concoction refresh after each item left inventory. This happened about 1300 items in and stopped the item movement. Trying again led to the same result. Logging out and exiting and starting fresh allowed it to continue.

How the heck do you debug this?

Unexpected error, debug log printed.
class java.lang.IllegalArgumentException: Comparison method violates its general contract!
java.lang.IllegalArgumentException: Comparison method violates its general contract!
	at java.util.ComparableTimSort.mergeHi(ComparableTimSort.java:835)
	at java.util.ComparableTimSort.mergeAt(ComparableTimSort.java:453)
	at java.util.ComparableTimSort.mergeCollapse(ComparableTimSort.java:374)
	at java.util.ComparableTimSort.sort(ComparableTimSort.java:182)
	at java.util.Arrays.sort(Arrays.java:537)
	at java.util.TimSort.sort(TimSort.java:178)
	at java.util.TimSort.sort(TimSort.java:173)
	at java.util.Arrays.sort(Arrays.java:659)
	at java.util.Collections.sort(Collections.java:217)
	at net.java.dev.spellcast.utilities.LockableListModel.sort(LockableListModel.java:152)
	at net.java.dev.spellcast.utilities.LockableListModel.sort(LockableListModel.java:143)
	at net.sourceforge.kolmafia.persistence.ConcoctionDatabase.refreshConcoctionsNow(ConcoctionDatabase.java:1393)
	at net.sourceforge.kolmafia.persistence.ConcoctionDatabase.refreshConcoctions(ConcoctionDatabase.java:1212)
	at net.sourceforge.kolmafia.request.GenericRequest.execute(GenericRequest.java:1517)
	at net.sourceforge.kolmafia.request.GenericRequest.run(GenericRequest.java:1407)
	at net.sourceforge.kolmafia.request.TransferItemRequest.run(TransferItemRequest.java:334)
	at net.sourceforge.kolmafia.request.ClosetRequest.run(ClosetRequest.java:289)
	at net.sourceforge.kolmafia.request.TransferItemRequest.runSubInstances(TransferItemRequest.java:168)
	at net.sourceforge.kolmafia.request.TransferItemRequest.run(TransferItemRequest.java:312)
	at net.sourceforge.kolmafia.request.ClosetRequest.run(ClosetRequest.java:289)
	at net.sourceforge.kolmafia.RequestThread.postRequest(RequestThread.java:278)
	at net.sourceforge.kolmafia.RequestThread.postRequest(RequestThread.java:241)
	at net.sourceforge.kolmafia.swingui.panel.ItemManagePanel$TransferListener.retrieveItems(ItemManagePanel.java:616)
	at net.sourceforge.kolmafia.swingui.panel.ItemManagePanel$TransferListener.initialSetup(ItemManagePanel.java:598)
	at net.sourceforge.kolmafia.swingui.panel.ItemManagePanel$PutInClosetListener.execute(ItemManagePanel.java:715)
	at net.sourceforge.kolmafia.swingui.listener.ThreadedListener.run(ThreadedListener.java:239)
	at net.sourceforge.kolmafia.RequestThread$SequencedRunnable.run(RequestThread.java:404)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
	at java.util.concurrent.FutureTask.run(FutureTask.java:262)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
	at java.lang.Thread.run(Thread.java:745)

 

[roippi]

Well the "contract" it's talking about is

if a.equals(b) then a.compareTo(b) == 0

and

if !a.equals(b) then a.compareTo(b) != 0

Also I'm guessing it should be invertible; a.compareTo(b) == -b.compareTo(a)

Looking at .equals() and .compareTo() in Concoction I wonder how this bug doesn't come up more often. For example this snippet in compareTo looks particularly sketchy:

		if ( this.name.startsWith( "steel" ) )
		{
			return -1;
		}
		else if ( o.name.startsWith( "steel" ) )
		{
			return 1;
		}

What if you can make a steely marble?

 

[Veracity]

Yeah, I just studied the contacts for equals, hashCode, and compareTo.

I notice that equals and hashCode all simply defer to the same method applied to the name field of the Concoction. The compareTo method uses compareToIgnoringCase on the name field - which means it is not "consistent to equals" - a bad (and unnecessary) thing.

The steel organ check was also unnecessarily time consuming (doing string comparison), had false matches for the "steely marble" and "steel sword", and did not return 0 when comparing two things with the same name.

Revision 14736 fixes all of the above. I've verified that I can refresh concoctions and it no longer throws an exception, but I couldn't make it happen on demand before, so that proves nothing - other than that I likely did not make things worse.

I'll mark this "Fixed".

 

[fronobulax]

Thank you. This was on my list but I was AFK for a day.