Problem sorting with Comparator.comparing()

[Carey Brown]

Here's an SSCCE for my issue. I need to do a sort of the Data.str field either in natural order or in reverse order depending on the value in Data.ascending. I managed to do this just fine in Data's compareTo() method, but now I'm trying to convert it to use Comarator. It mostly works but I can't figure out how to do the natural/reversed based on a value. Note that because the Data is sorted first by "ascending", by the time it needs to determine natural/reversed both arguments will be the same, either both natural or both reversed.

import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Random;

public class SortOfSort4
{
	public static void main( String[] args ) throws Exception
	{
		new SortOfSort4().run();
	}
	
	Comparator<Data> compData = Data.compIsAscending.thenComparing( Data.compStr );
	
	public void run() throws Exception
	{
		List<Data>		list = loadData();
		
		list.sort( compData );		// ** Does not sort descending
//		Collections.sort( list );	// ** WORKS
		
		System.out.println( "Group 'f' should be descending, 't' should be ascending" );
		for( Data d : list )
			System.out.println( d );
	}

	public List<Data> loadData() throws Exception
	{
		Random		rand = new Random();
		List<Data>	list = new ArrayList<>();
		for( int i=0 ; i < 25 ; i++ )
			list.add( new Data( rand.nextBoolean(), "" + (char)('A' + rand.nextInt(26)) ) );
		return list;
	}
	
	//======================================================================
	class Data
	implements Comparable<Data>
	{
		boolean			ascending;
		String			str;
		int				reverse;
		
		public Data( boolean ascending, String str )
		{
			this.ascending = ascending;
			this.str = str;
			reverse = ascending ? 1 : -1;
		}
				
		public String toString()
		{
			return String.format( "%s %s", ascending?"t":"f", str );
		}
		
		// ** WORKS
		public int compareTo( Data d2 )
		{
			int		result = Boolean.compare( ascending, d2.ascending );
			if( result == 0 )
				result = str.compareTo( d2.str ) * reverse;
			return result;
		}
		
		public boolean	isAscending() { return ascending; }
		public String	getStr() { return str; }
		public int		getAscendingInt() { return ascending ? 1 : 0; }
		
		// ** FIX :: MISSING LOGIC TO SORT STR DESCENDING CONDITIONAL ON "ascending"
	    static Comparator<Data>	compIsAscending = Comparator.comparingInt( Data::getAscendingInt );
	    static Comparator<Data> compStr = Comparator.comparing( Data::getStr );
	}
}

[Carey Brown]

Sorry Piet, your suggestion didn't work, that's why I whittled the problem down to this SSCCE, it is runnable as is.

[Piet Souris]

Yep. Well, it is a nasty one. I have a solution, but it is almost too ugly to show. So please don't shoot the messenger!

I could not make the Comparator static, since Data is an inner class. To get that Comparator anyway, I added an empty Constructor. And I cannot get the indentation up to standard, for some reason.

public List<Data> loadData() throws Exception
{
Random      rand = new Random();
List<Data>    list = new ArrayList<>();
for( int i=0 ; i < 25 ; i++ ) list.add( new Data( rand.nextBoolean(), "" + (char)('A' + rand.nextInt(26)) ) );
var sorted = list.stream().sorted(new Data().getDataComparator()).collect(toList());
var sorted2 = list.stream().sorted(Comparator.naturalOrder()).collect(toList());
var format = "%s - %s   -  %s%n";
for (int i = 0; i < list.size(); i++) out.format(format, list.get(i), sorted.get(i), sorted2.get(i));
return list;
}

//======================================================================
class Data
implements Comparable<Data>
{
boolean         ascending;
String          str;
int             reverse;
Comparator<Data> comp;

{
comp = (s, t) -> {
    Comparator<Data> comp1 = Comparator.comparingInt(Data::getAscendingInt);
    Comparator<Data> comp2 = (a, b) -> - a.str.compareTo( b.str ) * a.reverse;
    // copied from the compareTo method!
    return comp1.thenComparing(comp2).compare(s, t);
};
}

public Comparator<Data> getDataComparator() {
    return comp;
}

public Data( boolean ascending, String str )
{
    this.ascending = ascending;
    this.str = str;
    reverse = ascending ? -1 : 1;
}

public Data() {}

[Carey Brown]

Thanks for your help Piet. I was unsure of the (a,b)-> syntax and you pointed me in the right direction. This works now.

import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Random;

public class SortOfSort4
{
	public static void main( String[] args ) throws Exception
	{
		new SortOfSort4().run();
	}
	
	public void run() throws Exception
	{
		List<Data>		list = loadData();
		
		list.sort( Data.compData );	// ** WORKS NOW
//		Collections.sort( list );	// ** WORKS
		
		System.out.println( "Group 'f' should be descending, 't' should be ascending" );
		for( Data d : list )
			System.out.println( d );
	}

	public List<Data> loadData() throws Exception
	{
		Random		rand = new Random();
		List<Data>	list = new ArrayList<>();
		for( int i=0 ; i < 25 ; i++ )
			list.add( new Data( rand.nextBoolean(), "" + (char)('A' + rand.nextInt(26)) ) );
		return list;
	}
	
	//======================================================================
	// Note[1]: "compStr" must be called after "compIsAscending" returns zero
	// because "reverse" of only one of the arguments is used.
	class Data
	implements Comparable<Data>
	{
		boolean			ascending;
		String			str;
		int				reverse;
		
		public Data( boolean ascending, String str )
		{
			this.ascending = ascending;
			this.str = str;
			reverse = ascending ? 1 : -1;
		}
				
		public String toString()
		{
			return String.format( "%s %s", ascending?"t":"f", str );
		}
		
		// ** WORKS
		public int compareTo( Data d2 )
		{
			int		result = Boolean.compare( ascending, d2.ascending );
			if( result == 0 )
				result = str.compareTo( d2.str ) * reverse;
			return result;
		}
		
		public boolean	isAscending() { return ascending; }
		public String	getStr() { return str; }
		public int		getAscendingInt() { return ascending ? 1 : 0; }
		
	    static Comparator<Data>	compIsAscending = Comparator.comparingInt( Data::getAscendingInt );
	    static Comparator<Data> compStr = (a,b) -> a.str.compareTo( b.str ) * a.reverse; // see note[1]
		static Comparator<Data> compData = Data.compIsAscending.thenComparing( Data.compStr );
	}
}

[Mike Simmons]

I just replaced the original compData declaration with:

  Comparator<Data> compData = Data.compIsAscending
    .thenComparing((x,y) -> (x.isAscending() ? Data.compStr : Data.compStr.reversed()).compare(x, y));

Seems to work...

[ edit - ah, I now see it was already resolved.  I like the "compStr = (a,b) -> a.str.compareTo( b.str ) * a.reverse;" better, nice! ]

[Piet Souris]

Many ways lead to Rome, as they say.

@Carey:

I could not use static, since Data is not a static class. Well, java 10 that is. Is that from a later version, or did I just screw up?

[Carey Brown]

I'm using 16 and static works. <shrugs>