Is there any mistake in this Java code?

Question

class Creature {    
   private int yearOfBirth=10;

   public void setYearOfBirth(int year) {
      yearOfBirth = year;
   }

   void setYearOfBirth(Creature other) { 
      yearOfBirth = other.yearOfBirth; // is this correct it compiles fine 
   }

   int getYearOfBirth() { 
      return yearOfBirth;
   } 

   public static void main(String args[])
   {
      Creature c = new Creature();
      c.setYearOfBirth(89);

      Creature d = new Creature();
      c.setYearOfBirth(d);

      System.out.println(c.yearOfBirth);
   }
}

Is there any mistake in this code?

Is "other.yearOfBirth" wrong? My faculty says it is wrong but it works fine for me.

Answer 1

i detect nothing wrong.

the code works, because an instance or class can access private members of other instances of the same class. this is by design.

Answer 2

I see two "issues," though I hesitate to call them mistakes:

1. You're explicitly setting Creature c's age as 89, and then rewriting that age with the uninitialized default (!) of Creature d. If this is what you intend to do, then fine, but at the very least you're wasting a few cycles to set a value that you intend to throw out later.

2. You're possibly violating the JavaBeans naming conventions.

To explain the latter point: a lot of Java libraries and frameworks (notably JSP) rely on JavaBeans to treat the object as a component. I haven't dived deeply into the actual mechanisms used, but from what I've read it relies on Introspection of the JavaBeans class to infer properties and the types of those properties. By overloading your setter setYearOfBirth() to accept both an int and a Creature, you could throw off the Introspection. (See here for a decent introduction to JavaBeans.)

This is not a big deal -- its entirely possible that you won't use this class as a JavaBean, and if you do it's trivial to refactor it so it works. But your teachers would probably prefer something a little cleaner, like the following:

class Creature {    
   private int yearOfBirth=10;

   public void setYearOfBirth(int year) {
      yearOfBirth = year;
   }

   int getYearOfBirth() { 
      return yearOfBirth;
   } 

   public static void main(String args[])
   {
      Creature c = new Creature();
      c.setYearOfBirth(89);

      Creature d = new Creature();
      c.setYearOfBirth(d.getYearOfBirth());

      System.out.println(c.getYearOfBirth());
   }
}

Now all of your access to yearOfBirth comes via the public getter methods, which helps encapsulation, and will prevent the code from breaking if your main method moves to another class. (As Greg D correctly pointed out.)

Also, this has the added benefit of making the intent of your code clear, which becomes more and more important when you start writing code for others to maintain and modify.

Answer 3

No, there is no problem at all with it.

Look, it depends on the viewer opinion. But for a given context this code may be just perfect.

For some other context this may not be correct. So it depends of how is going to be used.

• Accessing a private member directly from another instance, is correct ( not always desirable though, for instance when you're subclassing ) that's why it is private in first place. You are saying "Hey, this is mine and I know how to use it"

• Using the default access modifier for the other two methods, says that your intention is they should not be used by other classes outside the package.

Probably the only thing I would add is to make the class final.

final class Creature   

If you want to make it inheritable you probably have to review the get/set for the yearOfBirth attribute, but they way it is is perfect to me.

NOW The most important thing here, is that you understand what each part of your code does, and how it affects its behavior.

You should no code just by luck ( sorry I don't know what's the correct expression for this ) but you should know what you're doing each time you type something, and how do you intend to be used.

Answer 4

You have to ask your faculty to explain why they think it's wrong (its perhaps a question of style, or even a misunderstanding), so you can learn from it.

Ultimately this person is going to impact your grades. This is an excellent opportunity to have a positive interaction with them. The more involved your teachers are with teaching you personally, the better your opportunity for mastering your subject will be.

If on the other hand when you're told something is wrong you go away privately and ask the general internet community, there is a risk that you'll be told you're right and you'll end up a false sense of superiority over your teachers which will be very counterproductive.

Answer 5

As written, it will work, as you discovered. I suspect that there's a fundamental misunderstanding at play, though.

My psychic powers tell me that your instructor expected code more like the following:

class Creature {    
   private int yearOfBirth=10;

   public void setYearOfBirth(int year) {
      yearOfBirth = year;
   }

   public void setYearOfBirth(Creature other) { 
      yearOfBirth = other.yearOfBirth;
   }

   public int getYearOfBirth() { 
      return yearOfBirth;
   } 
}

class Program {
   public static void main(String args[]) {
      Creature c = new Creature();
      c.setYearOfBirth(89);

      Creature d = new Creature();
      c.setYearOfBirth(d);

      System.out.println(c.yearOfBirth); // This will not compile
   }
}

The misunderstanding is that you've only created one class-- your main application class. This effectively makes yearOfBirth a sort of hybrid global value that you can access from your main method. In more typical designs, Creature is a class that is completely independent of your main method. In that case, you must only access Creature through its public interface. You would not be able to access its private field directly.

---

(Note to any pedants out there: Yes, I know I'm simplifying.)

Answer 6

yearofBirth is a private int. Therefore the call to other.yearOfBirth would presumably fail...

Answer 7

It's wrong because you're accessing a private member (you declared private int yearOfBirth) of another object although the class type is the same. You should use the public getter you defined instead: yearOfBirth = other.getYearOfBirth()