Apps Can't wrap my head around this comparator...

Valten1992 · Feb 27, 2012

One feature my app will have is the ability to sort a list of properties by how close they are to the users current position. I tried implementing a comparator but it does not seem to work.

In my attempt it takes two property objects, gets thier distance from the user and then compares that to each other, with the closest pushed closer to the front of the array. if anyone can see where I am going wrong with this I'll be grateful. i can do comparators with string arrays and the like but its been a long time since I did one for objects.

Code:

Collections.sort(res, new Comparator<Property>()
                        {

                            @Override
                            public int compare(Property p1, Property p2) {
                                
                                double la1 = p1.getLat();
                                double la2 = p2.getLat();
                                double lon1 = p1.getLon();
                                double lon2 = p2.getLon();
                                
                                
                                double dLat = Math.toRadians(lati - la1);
                                double dLon = Math.toRadians(longi - lon1);
                                
                                //determine distance from first point
                                double a =
                                        (Math.sin(dLat / 2) * Math.sin(dLat / 2) + Math.cos(Math.toRadians(la1))
                                        * Math.cos(Math.toRadians(lati)) * Math.sin(dLon / 2) * Math.sin(dLon / 2));
                                p1.setDist(a);
                                
                                double dLat2 = Math.toRadians(lati - la2);
                                double dLon2 = Math.toRadians(longi - lon2);
                                
                                //determine distance from second point
                                double a2 =
                                        (Math.sin(dLat / 2) * Math.sin(dLat / 2) + Math.cos(Math.toRadians(la2))
                                        * Math.cos(Math.toRadians(lati)) * Math.sin(dLon / 2) * Math.sin(dLon / 2));
                                p2.setDist(a2);
                                
                                int res2 = (int)p2.getDist();
                                int res = (int)p1.getDist();
                                        
                                return res2 - res;
                               
                            }    
                        });
                 PrototypeActivity.adapter.notifyDataSetChanged();
            }
        });

alostpacket · Feb 27, 2012

IMHO, you're trying to do too much at once. You need to separate the tasks/responsibilities.

First, determine the "distance" in a separate function like

calculateDistace(MyProperyObj obj)

and save that as a field/property to your property object.

Then, make the comparator an inner class or even a separate class. The comparator itself should be child's play at this point.

hth

Valten1992 · Feb 28, 2012

Thanks for that, I really need to learn to do that with my code more often.

Code:

class DistanceComparator implements Comparator<Property> {
    PrototypeActivity pa = new PrototypeActivity();
      public int compare(Property p1, Property p2) {
        
         pa.calculateDistance(p1);
         pa.calculateDistance(p2);
         
         if(p2.getDist()>p1.getDist())
         {
             return -1;
         }
         else if(p2.getDist() < p1.getDist())
         {
             return 1;
         }
         else
             return 0;
      }

alostpacket · Feb 28, 2012

^ much better

I probably wouldn't calculate the distance while in the comparator though but you got the idea

It's important the comparator work on a stable list, if you are calculating as you go, it could cause some strange sorting issues.

Valten1992 · Mar 12, 2012

alostpacket said:
^ much better

I probably wouldn't calculate the distance while in the comparator though but you got the idea

It's important the comparator work on a stable list, if you are calculating as you go, it could cause some strange sorting issues.

I hate to bring up this thread again but i am just noticing an issue here. One element that I sort seems to be in the wrong place while the rest of the values are sorted correctly

E.g 181,179,183,199 becomes 181,183,199,179

This is probably something obvious.

Code:

public class DistanceComparator implements Comparator<Property> {
    
    public int compare(Property p1, Property p2) 
    {

        if (p2.getDist() >= p1.getDist()) 
        {
            return -1;
        } else if (p2.getDist() <= p1.getDist()) 
        {
            return 1;
        } else
        {
            return 0;
        }
    }

}

Distance calculation is done in a different class now.

jonbonazza · Mar 12, 2012

Valten1992 said:
I hate to bring up this thread again but i am just noticing an issue here. One element that I sort seems to be in the wrong place while the rest of the values are sorted correctly

E.g 181,179,183,199 becomes 181,183,199,179

This is probably something obvious.

Code:

public class DistanceComparator implements Comparator<Property> { public int compare(Property p1, Property p2) { if (p2.getDist() >= p1.getDist()) { return -1; } else if (p2.getDist() <= p1.getDist()) { return 1; } else { return 0; } } }

Distance calculation is done in a different class now.

The problem is that in every conditional, you are testing for '=='. Change it to:

Code:

public class DistanceComparator implements Comparator<Property> {
    
    public int compare(Property p1, Property p2) 
    {

        if (p2.getDist() > p1.getDist()) 
        {
            return -1;
        } else if (p2.getDist() < p1.getDist()) 
        {
            return 1;
        } else
        {
            return 0;
        }
    }

}

Valten1992 · Mar 12, 2012

jonbonazza said:

The problem is that in every conditional, you are testing for '=='. Change it to:

Code:

public class DistanceComparator implements Comparator<Property> {
    
    public int compare(Property p1, Property p2) 
    {

        if (p2.getDist() > p1.getDist()) 
        {
            return -1;
        } else if (p2.getDist() < p1.getDist()) 
        {
            return 1;
        } else
        {
            return 0;
        }
    }

}

Whoops! That was a change I made trying to get it to work, the solution you suggest was the way I had it, it still does not work. In case it helps, here is the onClick event for the button

Code:

sort.setOnClickListener(new OnClickListener() {
            @Override
            public void onClick(View v) {

                Collections.sort(firstResults, new DistanceComparator());
                PrototypeActivity.adapter.notifyDataSetChanged();
                Toast t2 = Toast.makeText(getApplicationContext(),
                        "Results ordered from closest to farthest",
                        Toast.LENGTH_SHORT);
                t2.show();
            }
        });

jonbonazza · Mar 12, 2012

Can we see the series before sort and after sort?

Valten1992 · Mar 13, 2012

jonbonazza said:
Can we see the series before sort and after sort?

Refer to my 3rd post in this topic. The number is the distance value of the property object.

jonbonazza · Mar 14, 2012

Everything seems to be correct. Unless I am missing something, your problem lies somewhere else.

alostpacket · Mar 14, 2012

Agreed^

My guess would be that the values are changing or invalid as the lists sorts. What is the code that originally populates the list?

Basically, can getDist() return a different value on separate calls? What is important is that the lists here is stable. IE, are all the distance values calculated before you sort.

You might also try calling notifyDataSetInvalidated() on the adapter instead of notifyDataSetChanged() as you are not just changing the data behind a few items, but changing the order of the whole list.

Valten1992 · Mar 16, 2012

What is going on is that a txt file is parsed for data to populate an object with. With each line of the file representing a property object. At the end of every loop before the current property is added:

Dc is the distanceCalculator class and v is the Arraylist of properties

Code:

while ((s = b.readLine()) != null) {
 //Do stuff
.
.
.
.
//When your done
dc.calculateDistance(p);
                v.add(p);
                p = new Property();// set values to null for next loop

The calculateDistance class looks like this

Code:

public class DistanceCalc {
    
    double lati = 0;
    double longi = 0;
    
    // calculate distance between the user and the selected property
        public void calculateDistance(Property p) {
            double la = p.getLat();
            double lon = p.getLon();

            double dLat = Math.toRadians(lati - la);
            double dLon = Math.toRadians(longi - lon);

            // determine distance from first point
            double a = (Math.sin(dLat / 2) * Math.sin(dLat / 2) + Math.cos(Math
                    .toRadians(la))
                    * Math.cos(Math.toRadians(lati))
                    * Math.sin(dLon / 2) * Math.sin(dLon / 2));
            p.setDist(a);
        }
}

jonbonazza · Mar 16, 2012

When and where is the sort method called?

Valten1992 · Mar 20, 2012

jonbonazza said:
When and where is the sort method called?

When the button "sort" is clicked

Code:

sort.setOnClickListener(new OnClickListener() {
            @Override
            public void onClick(View v) {

                Collections.sort(firstResults, new DistanceComparator());
                PrototypeActivity.adapter.notifyDataSetInvalidated();
                Toast t2 = Toast.makeText(getApplicationContext(),
                        "Results ordered from closest to farthest",
                        Toast.LENGTH_SHORT);
                t2.show();
            }
        });

jonbonazza · Mar 20, 2012

Is the distance calculation and/or sorting being done in a seperate thread?

Valten1992 · Mar 27, 2012

jonbonazza said:
Is the distance calculation and/or sorting being done in a seperate thread?

Yes

jonbonazza · Mar 27, 2012

I bet that tge dataset is being accessed by multiple threads at one time. Have you tryed synchronizing the calculating thread?

Valten1992 · Apr 3, 2012

jonbonazza said:
I bet that tge dataset is being accessed by multiple threads at one time. Have you tryed synchronizing the calculating thread?

Sorry it took so long to get back.
I have tried synchronizing like you said but its still making at least one mistake in the list. Honestly, its not that big an issue compared to the other stuff I am doing now for the app anyway.

Apps Can't wrap my head around this comparator...

Valten1992

Newbie

alostpacket

Over Macho Grande?

Valten1992

Newbie

alostpacket

Over Macho Grande?

Valten1992

Newbie

jonbonazza

Android Expert

Valten1992

Newbie

jonbonazza

Android Expert

Valten1992

Newbie

jonbonazza

Android Expert

alostpacket

Over Macho Grande?

Valten1992

Newbie

jonbonazza

Android Expert

Valten1992

Newbie

jonbonazza

Android Expert

Valten1992

Newbie

jonbonazza

Android Expert

Valten1992

Newbie