1

I am using .toString to return a string representation of an object, i.e. 

jcb.engineMove(move.toString());

will produce e2e4.

What I am trying to do is to extract the text of this object (e2e4) as a string. After Googling I came across overriding the toString method so I came up with this:

@Override
public String toString() {
    String s = "";
    int newRank = getRank();
    int newFile = getFile();
    final Move move = new Move(rank, file, newRank, newFile);
    s+="" + move;
    return s;
}

My questions are fairly basic:

  1. is this the right approach
  2. How do I call this routine when trying to get the text of the object?
user1432365
  • 103
  • 1
  • 1
  • 7
  • 1
    That's a good approach, but creating a new `Move` instance inside `Move.toString()` is a very bad idea. – jahroy Jul 25 '12 at 22:34

7 Answers7

8

Overriding Object.toString is a good approach.

However, your current implementation makes a major mistake by creating a new Move object (see below).

To call the routine (once you fix it), do exactly what you're already doing:

jcb.engineMove(move.toString());

If toString() should only be used for debugging (as mre says) you could implement another method named getText that does the same thing.

IMPORTANT NOTE:

You should not be creating a new Move object inside its toString method.

This is a very bad idea (as mentioned by others).

Your toString method should simply build a string and return it.

jahroy
  • 22,322
  • 9
  • 59
  • 108
  • 1
    Thank you to everyone for their help and for pointing me in the right direction. I realise now that I am creating a new Move object inside a toString method which is not the best way forward. I will write a toString method in the Move class that will return a string of the move object. I have learned a lot in the last 15mins - excellent site and great help. – user1432365 Jul 25 '12 at 22:49
3

is that toString() implemented in the Move class? if yes then what I see is an infinite loop. And... I don't really understand why you are creating a new instance of a Move class.

Anyway, to generate the string representation in the Move class try with something like this:

public class Move {

  @Override
  public String toString() {
    StringBuilder builder = new StringBuilder();
    builder.append(rank).append(file);
    builder.append(newRank).append(newFile);
    return builder.toString();
  }

}

Then if you want to get the string representation what you are actually doing (jcb.engineMove(move.toString());) isn't a bad approach.

Alonso Dominguez
  • 7,750
  • 1
  • 27
  • 37
  • 1
    Much simpler would be to do `return rank+file+newRank+newFile;`. The compiler will automatically convert to using StringBuilder. – Steve Kuo Jul 25 '12 at 23:50
  • that would work as long as one of those values was a string, but if you take a look to the question you'll find that all of them are ints so your code wouldn't even compile. Regarding the use of `StringBuilder`: I was just trying to give some good practice to the OP, that would help him to avoid some kind of mistakes and make the code more efficient. – Alonso Dominguez Jul 25 '12 at 23:55
2

the use of Object#toString should be limited to debugging.

mre
  • 43,520
  • 33
  • 120
  • 170
  • yes, I read that on my travels when researching how to return the text of an object. I am new to Java and learning as I go. Thanks – user1432365 Jul 25 '12 at 22:22
  • 3
    And you _definitely_ should NOT be creating a new Move object inside of `Move.toString`! – jahroy Jul 25 '12 at 22:33
2

I hope this is not code of toString() method in Move class. The reason of my fear is that you are creating Move object inside of it and invoke recursively toString() method by s+="" + move; (this is same as s+=move.toString()).

Pshemo
  • 122,468
  • 25
  • 185
  • 269
1

  1. To overwrite the toString() method is the common and right way to implement a custom textual representation of an object. You will find this procedure throughout literature and documentation.

  2. In Java (like it is the case in other languages like C#) the toString() method is defined in the object type which means that every object in Java has this method. If your custom object (which inherits from class object) overwrites the toString() method then your base class provides a new implementation of this method which hides/omits the toString() method from the super class.

That means when you define a custom toString() method in your custom class A a call to an instance of that type (say it's a) a.toString() would result in calling your implementation.

marc wellman
  • 5,808
  • 5
  • 32
  • 59
1

  1. I would probably not use toString() in this case because it appears that you are simply repeating the logic that is in the Move class. In order to add any other details, I have one question: to what class are you adding this toString() method?

  2. You call this method just like any other method. First you need an instance of an object to call it with:

    someObj.toString();

To give any more details, I need an answer to the previous question.

Code-Apprentice
  • 81,660
  • 23
  • 145
  • 268
1

As mre said, you should not use toString() for something that your code depends on to function. Now, what are you trying to accomplish? Can you give any code from these classes? I would think your engineMove method should take a Move object, not a String. If you can give some more details, we might be able to steer you in a better direction.

Also, be careful with that code that you have. Why do you need to create a new Move object that takes up time and resources inside the toString()? toString() should operate on an instance of the class, so you shouldn't need to create a new one but even more so, using s+="" + move; will implicitly call toString() on the new Move object, which will call it again on a new Move object...

Carl
  • 905
  • 5
  • 9