Method to compare two author objects from author class

Question

I'm writing a class method to compare two author objects fields.

 public int compareTo(Author b) {

    int i = -2;
    String afn = this.firstName;
    String aln = this.lastName;
    String bfn = b.firstName;
    String bln = b.lastName;

    if(afn.equals(bfn)) { i++; }
    if(aln.equals(bln)) { i++; }
    return i;
  } 

However when I pass through an author object with different strings it still returns zero. Is there something wrong with my approach? Also is "this" necessary for the method?

Here's the class.

public class Author {


    String firstName = TITLE_NOT_SET;
    String lastName = TITLE_NOT_SET;
    int birth = YEAR_NOT_SET;
    int death = YEAR_NOT_SET;


  public Author(String ln, String fn) {

    String lastName = ln; 
    String firstName = fn;
  }


  public int getDeath() {

    return death;
  }

  public int getBirth() {

    return birth;
  }

  public void setDates(int b) {

    if(b > 2018 || b < -2000) { b = birth; }
    birth = b;
  }  

  public void setDates(int b, int d) {

    if(b > 2018 || b < -2000) { b = birth; }
    if(d < b) { d = death; }
    birth = b;
    death = d;
  }

  public int compareTo(Author b) {

    int i = -2;
    String afn = this.firstName;
    String aln = this.lastName;
    String bfn = b.firstName;
    String bln = b.lastName;

    if(afn.equals(bfn)) { i++; }
    if(aln.equals(bln)) { i++; }
    return i;
  }

  public String toString() {

     return lastName + ", " + firstName;
  }

}

Answer 1

You have a few errors in your code. The immediate one causing your problem is that your constructor is shadowing the instance variables so they never change when constructed as you would see when testing the toString methods of your class.

public Author(String ln, String fn) {
    String lastName = ln; //Remove 'String'
    String firstName = fn; //Remove 'String'
}

Adhering to my style, I would have written the constructor as such:

public Author(String lastName, String firstName) {
    this.firstName = firstName; //Using the 'this' makes it clear to me exactly what I wish to happen...
    this.lastName = lastName;
}

'Shadowing is where a variable of a higher scope is blocked because it shares its name with one in a closer scope (in practice usually a parameter, but it could be one declared there). Because they both share the same name the one with the closest scope is used in order to mitigate ambiguity, you can use the this keyword to access the instance scope, or a ClassName to access a static variable. The main thing to remember is that when declaring variables, the compiler won't necessarily tell you that you've shadowed a variable because it may be intentional so if you reuse names make sure that you're accessing the one within the scope you wish. Maybe more help here...