41

A lead developer on my project has taken to referring to the project's toString() implementations as "pure cruft" and is looking to remove them from the code base.

I've said that doing so would mean that any clients wishing to display the objects would have to write their own code to convert the object to string, but that was answered with "yes they would".

Now specifically, the objects in this system are graphic elements like rectangles, circles, etc and the current representation is to display x, y, scale, bounds, etc...

So, where does the crowd lie?

When should you and when shouldn't you implement toString?

Erick Robertson
  • 32,125
  • 13
  • 69
  • 98
Allain Lalonde
  • 91,574
  • 70
  • 187
  • 238
  • Wow, I was expecting more argument. – Allain Lalonde Jul 21 '09 at 22:26
  • 10
    Seems like there is a consensus for keeping them in. I wonder what is the motivation behind your lead developer wanting to remove them? Performance? Maintenance? Refactoring troubles? If we could understand his motivation better that would probably lead to a better argument. – Gregory Mostizky Jul 22 '09 at 07:10
  • i think best-practices is the reason, it has bad performance as best-practice. – IAdapter Jul 27 '09 at 12:16
  • 5
    Your lead is insane. If this is a reflection of your lead's typical thinking, you might consider looking for another job... – Chris Kessel Aug 12 '09 at 17:26
  • Umm... how did this guy get to become a "lead"? He needs to be fired... or you need to find a new job. – Vivin Paliath Jun 14 '10 at 22:39

24 Answers24

68

What harm do they do? Why remove them if you have them? I find toString() extremely useful when emitting debugging statements.

Personally, I would always err on the side of having a workable toString() method. So little work to write.

Andrew Ferrier
  • 16,664
  • 13
  • 47
  • 76
djna
  • 54,992
  • 14
  • 74
  • 117
  • 23
    toString() for debugging is so useful, that Eclipse allows you to provide your own "toString()" override for classes in libraries that you can't edit. The feature is called "Detail Formatters". – Harold L Jul 21 '09 at 19:40
  • HaroldL: Could you elaborate on how to use that feature? I'm an Eclipse user and this is the first I've ever heard of it, so I'll definitely be looking for it. – Thomas Owens Jul 21 '09 at 19:46
  • 1
    A piece of debug is like Gold for someone else picking it up in the future... leave them in they don't do any harm. – hookenz Jul 21 '09 at 21:30
  • Thomas: Look for "New detail formatter" popup menu item in the Variables pane. – robinr Jul 21 '09 at 22:49
  • 2
    Detail Formatter: http://www.javalobby.org/java/forums/t16350.html. Window->Preferences->Java->Debug->Detail Formatters – KitsuneYMG Jul 22 '09 at 13:00
  • great idea, i would do special project called ProjectNameDetailFormatter, because i think its easier to write them once. – IAdapter Jul 27 '09 at 12:18
  • There is one possible downside of having toString methods and that comes about because they often aren't written with performance in mind. They can be super handy which can result in developers logging them all over the place, causing performance issues. This is more a case of naughty programmers will do naughty things though... – Programming Guy Mar 09 '12 at 05:57
  • @Peter - perhaps, but I fear that in the absence of toString() the naughty will just write getA() + getB() etc. which is possibly worse. – djna Mar 09 '12 at 17:50
35

Removing well-written (or even halfway decently written) toString() methods is pure insanity, IMO. Yes, I am often too lazy to write these (as often the objects don't end up having them used anyway), but they are extremely handy to have.

I really can't think of a good reason to want to get rid of these.

jsight
  • 27,819
  • 25
  • 107
  • 140
  • Yes, exactly. There are no good reasons for removing the existing toString() methods. Moreover, even the ones that are not complete can be useful when debugging. – b.roth Jul 21 '09 at 21:44
18

I've always made sure that my classes implemented toString.

It provides a simple way of debugging the current state of the class when I'm debugging and when I'm logging errors, I can include it into my log messages.

z -
  • 7,130
  • 3
  • 40
  • 68
15

I'd keep the toString() implementations. They are invaluable when it comes to debugging, and they can make for good alt text for graphical components.

jjnguy
  • 136,852
  • 53
  • 295
  • 323
12

I would argue the opposite that toString() should be overriden judiciously. The default toString() implementation is very uninformative and basically useless. A good toString() implementation can give a developer a very useful view of the contents of the object at a glance. You may not have to put everything in there but at least the important stuff. I think your lead developer should be actually coding and adding features rather than worrying about "cruft".

Mike C.
  • 1,035
  • 2
  • 8
  • 13
11

I would only implement it for more complex objects where client code doesn't care about the fine grained details of the object state, but rather care about some more human understandable, sense making message, that summarizes what is going on, state wise...

For everything else, like JavaBeans, I would expect client code to throw my object into a ToStringBuilder method or similar, if it needs to do low-level debugging.

ToStringBuilder.reflectionToString(myObject);

Or client code should just call standard property getters and log the way they like...

Panther
  • 3,312
  • 9
  • 27
  • 50
raoulsson
  • 14,978
  • 11
  • 44
  • 68
  • That's a neat trick, but I would have to guess that most people have never heard of that. Also, consider what is more work: downloading the Apache Lang library and adding it to your program, or simply using the toString method that is already there and ready to use? – Kevin Panko Jul 21 '09 at 23:42
7

In general, toString() is good stuff. In particular, it is quite useful for debugging.

Implementing toString() is not without costs and risks. Like all code, toString() implementations must be maintained with the rest of the code. This means keeping toString() in sync with class fields. For example, when a field is added or removed, toString() should be updated appropriately (you should be doing this already for methods like hashCode() and equals()).

Implementing toString() incurs risks as well. For example, suppose that two classes in your system have references to instances of the other (a bi-directional link), then an invocation of toString() could lead to a stack overflow due to unbounded recursion as the toString() implementation in each class invokes the toString() implementation of the other class.

If your system has a significant number of out-of-sync toString() methods, or methods that cause bugs like stack overflows, then your colleague might have a reasonable point. Even in such a case, I would simply comment-out the buggy toString() methods and leave them in the code. Each toString() method could be uncommented and updated individually as needed in the future.

Greg Mattes
  • 33,090
  • 15
  • 73
  • 105
6

I always auto-generate toString() methods for all my POJOs, DTOs and/or any object that holds persistent data. For private internal properties good logging practice should do the trick.

Always remember to replace in toString methods passwords and other sesitive info with [Omitted] (or something similar of top secrete nature)

MatBanik
  • 26,356
  • 39
  • 116
  • 178
4

Well, he does stramge thing.

I can not say toString() is too useful. For presentation you will need other tools.

But toString() is quite useful for debugging, because you could see the contents of collections.

I do not understand why remove it if it is written already

Igor Shubovych
  • 2,760
  • 1
  • 20
  • 11
4

I think the answer depends on how complicated your toString() methods are, how much work they require to maintain, and how often they get used. Assuming you use toString() often for logging and debugging it doesn't make much sense to remove these methods. But if they are rarely used and require a lot of work to maintain each time something changes in your code, then there is perhaps a valid argument for getting rid of all or some of the toString() methods.

You mentioned something about clients needing to display these objects. From this I'm guessing your code is or contains some kind of library or API that other developers will use. In this case I highly recommend you maintain useful toString() implementations. Even if you don't do a lot of logging and debugging, your clients might and they will definitely appreciate having useful toString() methods that they don't have to write and maintain themselves.

Cosmin Stejerean
  • 1,348
  • 9
  • 9
3

+1 Mike C

Over and above its usefulness for debugging, the toString() is an invaluable tool to understand the class author's perspective of the instance.

FWIW, if the output of the toString differs from what you expect to see (courtesy spec docs) you will know rightaway something went seriously wrong.

2

Personally, I implement them when I'm going to use objects in a JList, JTable, or other structure that uses toString() OR when I'm debugging (yes, eclipse has debug formatters but toString() is easier).

Perhaps you can fire back that many JDK classes have toString(). Should they be removed as well? ;)

basszero
  • 29,624
  • 9
  • 57
  • 79
2

I would say you should implement toString if that is an expected use case or requirement, to display the object as a string representation (either in logs, on the console, or some kind of display tree).

Otherwise, I agree with the developer - every time you change something, the toString can break. You may have to be careful about nulls, etc.

Many times, though, it is in fact used in debugging or in logging, so it isn't obvious that they should be left out at all.

I agree with jsight that if they are already written, and written decently, leave them in at least until they get in the way (such as you actually add a field to a class).

Yishai
  • 90,445
  • 31
  • 189
  • 263
2

For debugging purposes, no one can beat toString. It's practical both in a debugger, and in simple debug prints. Make sure it displays all the fields that your equals and hashCode methods are based on, if you override those as well!

For display to end users, I wouldn't use toString, though. For that, I think it's better to write another method, that does the proper formatting, and i18n if you need that.

jqno
  • 15,133
  • 7
  • 57
  • 84
2

It makes good sense because you always have problems with toStrings showing too little or too much information.

It may make sense to your team to use the ToStringBuilder in Jakarta Commons Lang instead:

System.out.println("An object: " + ToStringBuilder.reflectionToString(anObject));

which introspects the object, and prints out the public fields.

http://commons.apache.org/lang/api-2.3/org/apache/commons/lang/builder/ToStringBuilder.html

Thorbjørn Ravn Andersen
  • 73,784
  • 33
  • 194
  • 347
2

I've said that doing so would mean that any clients wishing to display the objects would have to write their own code to convert the object to string, but that was answered with "yes they would".

This is not a question that can be answered in isolation ... you should ask the clients (or the people writing them) what they think of the idea. If I was using a Java library and relying on its toString() overloads for debugging, I'd be rather annoyed if the library's developers decided to purge them.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
2

To be fair, said developer here, but not lead developer in any sense.

The original issue is not necessarily about toString(), but about a second method paramString: "With all of its string concatenation and null value checking, paramString is a bug magnet."

See http://code.google.com/p/piccolo2d/issues/detail?id=99

2

I would definitely keep the toString() implementations, particularly for debugging purposes. As a primarily C++ developer, I wish things were as easy in C++ as they are in Java in this respect (operator overloading can be a pain).

Ryan Zink
  • 523
  • 1
  • 4
  • 11
2

If there is a problem with the existing toString() implementations, the developer should fix the problem. Saying the current implementations are all "pure cruft" and removing them is actively doing harm, unless the existing toString() methods are uncommonly poorly written.

I would strongly discourage the developer from removing any functioning toString() method.

Eddie
  • 53,828
  • 22
  • 125
  • 145
1

implement always :) As mentioned above, it is invaluable for debugging.

Larry Watanabe
  • 10,126
  • 9
  • 43
  • 46
1

It is good for debugging purposes. But if you want to display given object as a string to the end user you should never use toString() implementation but provide custom method for that.

So regarding

I've said that doing so would mean that any clients wishing to display the objects would have to write their own code to convert the object to string, but that was answered with "yes they would".

I agree with your team lead. If you want to display object to any client use custom implementation. If you want to use it for debugging purposes use toString().

jan
  • 273
  • 1
  • 6
0

Although toString() methods are very useful for debugging value classes, it can be argued they are not useful for entity classes.

Community
  • 1
  • 1
Raedwald
  • 46,613
  • 43
  • 151
  • 237
0

I figured I'd weigh in with a more modern perspective given that this question is now almost 10 years old.

toString is obviously helpful for debugging - you need look no further than all of the other answers here which attest to that - but debugging information isn't business logic.

Having a toString method in every single class is visual clutter which obscures the useful behaviour of the class. We also need to remember to maintain it - either manually or by regenerating the method from our IDE - every time we change the class fields.

So what can we do to address these problems without removing the method altogether? The answer is to automatically generate it. Project Lombok's @ToString annotation can automatically generate a toString method for you at compile-time which includes any combination of fields that you choose.

Basic sample usage:

@ToString
public class Foo {
    private int i = 0;
}

Which will become equivalent to the following at compile-time:

public class Foo {
    private int i = 0;

    public String toString() {
        return "Foo(i=" + this.i + ")";
    }
}
Michael
  • 41,989
  • 11
  • 82
  • 128
-1

We're getting a ConcurrentModificationException thrown out of one of our toString() methods, so there's an occasional drawback. Of course it's our own fault for not making it synchronized.

David Plumpton
  • 1,929
  • 23
  • 31