is this correct way of implimenting comparable interface

[Samanthi perera]

this is my main class

package Annotation;

import java.util.*;
import org.omg.PortableInterceptor.SYSTEM_EXCEPTION;


public class coll {

     public static void main(String args[]) {

        ArrayList<String> stuff = new ArrayList<String>();
      
        dog d1=new dog();
        dog d2=new dog();

        d1.setLegs(4);
        d1.setName("aa");

         d2.setLegs(4);
        d2.setName("aa");


        System.out.println("sssssssssssssssssssssss "+d1.compareTo(d2));


    }

}

this is my bean class

package Annotation;

public class dog implements Comparable {

    private int legs;

    public int getLegs() {
        return legs;
    }

    public void setLegs(int legs) {
        this.legs = legs;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }
    private String name;

    public int compareTo(Object o) {
        int result = 0;
        dog d=(dog)o;
        if (legs == d.getLegs()) {
            result = 1;
        }
        return result;
    }

   
}

[Ireneusz Kordal]

No it isn't.

see api:
http://java.sun.com/j2se/1.4.2/docs/api/java/lang/Comparable.html

int compareTo(Object o)
.....
Returns: a negative integer, zero, or a positive integer as this object is less than, equal to, or greater than the specified object.

You must return 0 when two objects are equal,
but the code returns 1 where two dogs have the same number of legs and 0 in other cases.

Try this:

 public int compareTo(Object o) {  
         dog d=(dog)o;  
         return legs - d.getLegs();
     }  

[Samanthi perera]

i change my class to this

package Annotation;

public class dog implements Comparable<dog> {

    private int legs;

    public int getLegs() {
        return legs;
    }

    public void setLegs(int legs) {
        this.legs = legs;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }
    private String name;

    public int compareTo(dog o) {

        final int BEFORE = -1;
        final int EQUAL = 0;
        final int AFTER = 1;
        int result=0;

        if(legs>o.getLegs()){
            result=AFTER;
        }
        if(legs==o.getLegs()){
            result=EQUAL;
        }
        if(legs<o.getLegs()){
            result=BEFORE;
        }

        return result;
    }
}

is this ok?

[pete stein]

Samanthi perera wrote:
is this ok?

What happens when you test it?

[Samanthi perera]

this is my main class

package Annotation;

import java.util.*;
import org.omg.PortableInterceptor.SYSTEM_EXCEPTION;


public class coll {

     public static void main(String args[]) {

        ArrayList<dog> stuff = new ArrayList<dog>();
      
        dog d1=new dog();
        dog d2=new dog();
        dog d3=new dog();

        d1.setLegs(6);
        d1.setName("dd");

         d2.setLegs(3);
        d2.setName("bb");

         d2.setLegs(1);
        d2.setName("aa");


       stuff.add(d1);
       stuff.add(d2);
       stuff.add(d3);

       Collections.sort(stuff);

       System.out.println("SORTTTTTTTTTTTTTTTTTTTT "+stuff.get(0).getName());
       System.out.println("SORTTTTTTTTTTTTTTTTTTTT "+stuff.get(1).getName());
        System.out.println("SORTTTTTTTTTTTTTTTTTTTT "+stuff.get(2).getName());

    }

}

when i run this it gives

SORTTTTTTTTTTTTTTTTTTTT null
SORTTTTTTTTTTTTTTTTTTTT aa
SORTTTTTTTTTTTTTTTTTTTT dd
BUILD SUCCESSFUL (total time: 4 seconds)

why it gives null for first item..........

[Nicola Garofalo]

You did not set d3 name, that's why it's null.
You set two times d2's name.
Look at your code

        d1.setLegs(6);
        d1.setName("dd");

         d2.setLegs(3);
         d2.setName("bb");

        d2.setLegs(1);
        d2.setName("aa");

then, as you sorted the three dogs, d3 happens to be the first one in your arraylist. and when you get its name it is null.
I mean:

stuff.get(0).getName() == null

[Samanthi perera]

that means this implimentation is ok?

[Nicola Garofalo]

i was answering to your last question, if i did not get it wrong: why it gives null for the first item.

As you wrote your compareTo method it happens that a dog is placed before another dog in your List if it has less legs, is that what you meant?

When you construct a dog it has 0 legs.

then when your code is executed:

d1.setLegs(6);  
d1.setName("dd");  
  
d2.setLegs(3);  
d2.setName("bb");  
  
d2.setLegs(1);  
d2.setName("aa");

it happens that

d1 has 6 legs
d2 has 1 leg
d3 has 0 legs

then the order is

d3
d2
d1

I hope i replied to your question

[Samanthi perera]

no , i change the main class as below

package Annotation;

import java.util.*;
import org.omg.PortableInterceptor.SYSTEM_EXCEPTION;

public class coll {

    public static void main(String args[]) {

        ArrayList<dog> stuff = new ArrayList<dog>();

        dog d1 = new dog();
        dog d2 = new dog();
        dog d3 = new dog();

        d1.setLegs(6);
        d1.setName("dd");

        d2.setLegs(4);
        d2.setName("bb");

        d3.setLegs(2);
        d3.setName("aa");


        stuff.add(d1);
        stuff.add(d3);
        stuff.add(d2);

        Collections.sort(stuff);

        System.out.println("SORTTTTTTTTTTTTTTTTTTTT " + stuff.get(0).getName());
        System.out.println("SORTTTTTTTTTTTTTTTTTTTT " + stuff.get(1).getName());
        System.out.println("SORTTTTTTTTTTTTTTTTTTTT " + stuff.get(2).getName());

    }
}

now it gives result here

SORTTTTTTTTTTTTTTTTTTTT aa
SORTTTTTTTTTTTTTTTTTTTT bb
SORTTTTTTTTTTTTTTTTTTTT dd

so i think this implimentation is correct,because it gives result as expected.
i need to know is this is good implimentation?

[Nicola Garofalo]

I can only tell you that now it's correct.
In my opinion it's up to you to decide whether it is also good or not. If in your domain exist dogs with more or less than 4 legs, it could be also good.

[David Newton]

Depends on what you mean by "good". I'd say "no", because there's an amazing amount of code, several poor naming issues, and some miscellaneous improvements. See embedded comments, then compare.

coll.java

// Package names should begin with a lower-case letter, and make sense.
// There are no annotations anywhere in this code; it's in the wrong package.
package Annotation;

// Particularly when learning, I find it best to use only explicit imports.
import java.util.*;

// Why on EARTH is this in there? It's neither used nor remotely relevant.
import org.omg.PortableInterceptor.SYSTEM_EXCEPTION;

// Class names should start with a capital letter, and be meaningful.
class coll {

    public static void main(String args[]) {

        // Code to interfaces, not to implementations.
        // Use meaningful variable names, not random words.
        ArrayList<Dog> stuff = new ArrayList<Dog>();
        
        // Why all this effort?
        Dog d1 = new Dog();
        Dog d2 = new Dog();
        Dog d3 = new Dog();

        d1.setLegs(6);
        d1.setName("dd");

        d2.setLegs(4);
        d2.setName("bb");

        d3.setLegs(2);
        d3.setName("aa");


        stuff.add(d1);
        stuff.add(d3);
        stuff.add(d2);

        Collections.sort(stuff);

        System.out.println("SORTTTTTTTTTTTTTTTTTTTT " + stuff.get(0).getName());
        System.out.println("SORTTTTTTTTTTTTTTTTTTTT " + stuff.get(1).getName());
        System.out.println("SORTTTTTTTTTTTTTTTTTTTT " + stuff.get(2).getName());
    }
}

TestComparableDog.java (my version)

package collections;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;

public class TestComparableDog {

    // Note that a better way to test this would be to create a list
    // of random dogs with a random number of legs, then iterate
    // over the list after sorting, making sure that each next dog
    // has the same or more legs than the previous dog.
    public static void main(String args[]) {
        List<Dog> dogs = Arrays.asList(new Dog("dd", 6), new Dog("aa", 2), new Dog("bb", 4));
        Collections.sort(dogs);
        for (Dog dog : dogs) {
            System.out.println(dog);
        }
    }

}

dog.java

// See previous package name comment.
package Annotation;

// See previous class name comment.
public class dog implements Comparable<dog> {

    // A convenience constructor would be very helpful.

    private int legs;

    public int getLegs() {
        return legs;
    }

    public void setLegs(int legs) {
        this.legs = legs;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }
    private String name;

    // This is a *lot* of code to do this compare...
    public int compareTo(dog o) {

        final int BEFORE = -1;
        final int EQUAL = 0;
        final int AFTER = 1;
        int result=0;

        if(legs>o.getLegs()){
            result=AFTER;
        }
        if(legs==o.getLegs()){
            result=EQUAL;
        }
        if(legs<o.getLegs()){
            result=BEFORE;
        }

        return result;
    }
}

Dog.java (my version)

package collections;

public class Dog implements Comparable><Dog> {

    private String name;
    private int legs;

    public Dog(String name, int legs) {
        this.legs = legs;
        this.name = name;
    }

    public int compareTo(Dog otherDog) {
        return legs - otherDog.getLegs();
    }

    @Override
    public String toString() {
        return super.toString() + "{" +
                "name='" + name + '\'' +
                ", legs=" + legs +
                '}';
    }

    public int getLegs() {
        return legs;
    }

    public void setLegs(int legs) {
        this.legs = legs;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

}