Sorting wih Comparator dont work ?

[Frank Jacobsen]

I have a class that sorts with comprator, but i dosent work !

Any good ideas, look at my code ?

Is there a better way to sort my list with objects ?

   

package dk.jyskebank.jyskeinvest.backingbean.todayshighandlow;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.Iterator;
import java.util.List;
import dk.jyskebank.j2ee.core.dao.DataAccessObject;
import dk.jyskebank.j2ee.core.exception.JyskeSystemException;
import dk.jyskebank.jyskeinvest.backingbean.BackingBeanHelper;
import dk.jyskebank.jyskeinvest.backingbean.BackingBeanHelperImpl;
import dk.jyskebank.jyskeinvest.dao.JdbcDaoFactory;
import dk.jyskebank.jyskeinvest.dao.funds.FundDao;
import dk.jyskebank.jyskeinvest.domain.funds.Fund;

public class TodaysHighAndLowBackingBean  implements Comparator {
	
	private BackingBeanHelper helper = null;
	private final FundDao fundDao;
	protected List funds;
	
	public TodaysHighAndLowBackingBean() {
		this(new BackingBeanHelperImpl(), new JdbcDaoFactory().getFundDao());
	}

	protected TodaysHighAndLowBackingBean(BackingBeanHelper helper, FundDao dao) {
		this.helper = helper;
		this.fundDao = dao;
	}
	
	
	public String getRequestLanguage() {
		return helper.getRequestLanguage();
	}
	
	protected  Fund[] getFunds() {
		if (funds == null) {
			try {
				this.funds = fundDao.getAllFunds();
			} catch (Exception e) {
				throw new JyskeSystemException("Auto generated exception", e);
			} finally {
				((DataAccessObject) fundDao).close();
			}
		}
		
		int antalFunds = funds.size();
		
		Fund fundArray[] = new Fund[antalFunds];
		int i = 0;
		
		for (Iterator iterFund = funds.iterator(); iterFund.hasNext();i++) {
			Fund fund = (Fund) iterFund.next();
			fundArray[i] = fund;
			System.out.println("FJA fund.getNavToday() " + fund.getNavToday());
		}
						
		Arrays.sort(fundArray, new TodaysHighAndLowBackingBean());

		return fundArray;
	}

	public int compare(Object o1, Object o2) {

		double navToDay1 = ( (Fund) o1).getNavToday();

		double navToDay2 = ( (Fund) o2).getNavToday();

		if( navToDay1 > navToDay2 )

		return 1;

		else if( navToDay1 < navToDay2 )

		return -1;

		else

		return 0;
	
	}
	
}

Frank  :banghead:

[Daesung Park]

At a glance, creating a new instance in line 60 is problematic.

There are many ways to implement it correctly.
One solutin is to remove "implements Comparator" from TodaysHighAndLowBackingBean, and

 // line 60
Arrays.sort(fundArray, new Comparator() {
   public int compare(Object o1, Object o2) {
    ...
   }
});

[Rob Spoor]

Daesung Park wrote:At a glance, creating a new instance in line 60 is problematic.

Although it is unnecessary, it shouldn't create any problems. And replacing it with

Arrays.sort(fundArray, this);

will work just as fine.

Frank, what is the value of navToDay1 and navToDay2? Are you sure those values are correct?

You might also want to consider using

Double.compare(navToDay1, navToDay2)

[Frank Jacobsen]

Arrays.sort(fundArray, this); i will try this monday when i back on work....

The value are doubles and contains in the range -8.00 and 10.00

I get a List back in the fundDao.getAllFunds(), i dont want to implement the Comparable in fundDao.....

Is there a more effective way to do this, in sted off looping true the list, and put it into a Array and sort the Array ?



Frank

[Paul Clapham]

Frank Jacobsen wrote:Is there a more effective way to do this, in sted off looping true the list, and put it into a Array and sort the Array ?

I'm not sure what this question really means. There's Collections.sort(List, Comparator) which sorts a List -- so if you used that, you wouldn't have to copy the List to an array. You could just sort it in place. Or there's List.toArray() which produces an array from a List -- so if you used that, you wouldn't have to write your own code to copy the List to an array.

[Frank Jacobsen]

So you mean that this will work:

this is implementing Comparator

this.funds is a list.

// see the above code

  


   Collections.sort( this.funds , this) ; 
  

  

Frank

[Paul Clapham]

It should work. Of course you would have to change your method to return a List instead of an array of Funds.

[Frank Jacobsen]

Thanks a LOT