0

I am trying to determine a best practice when writing code to get the string representation of a custom class.

Imagine we have the following:

public class DoubleParameter
{
    public double Value { get; set; }
    public string Description { get; set; }
    public string Units { get; set; }
}

And we want the ability to get a string representation of the class for debugging purposes. Regarding code readability/maintainability and best practices, I'm evaluating three options

  1. An Inline Property
  2. A Custom Method
  3. Overriding ToString()

Most of these are very similar from the compiler's point of view - but are there any objective reasons to prefer any particular option in terms of readability/maintainability? Or is it a matter of personal preference?

Examples of use:

// Option 1 - Inline Property
public string ReadableValue => 
    $"{this.Description} => {this.Value.ToString("F2")} ({this.Units})";
// example usage: Console.WriteLine(myVar.ReadableValue);

// Option 2 - Custom Method
public string ToReadable() =>
    $"{this.Description} => {this.Value.ToString("F2")} ({this.Units})";
// example usage: Console.WriteLine(myVar.ToReadable());

// Option 3 - Overriding ToString()
public override string ToString() =>
    $"{this.Description} => {this.Value.ToString("F2")} ({this.Units})";
// example usage: Console.WriteLine(myVar);
LarsTech
  • 80,625
  • 14
  • 153
  • 225
Ricardo Fuente
  • 33
  • 1
  • 1
    Opinions are against the rules around here. – LarsTech Dec 03 '18 at 21:56
  • 1
    The `ToString` implementation has the advantage of making debugging way easier. And, oh yeah, no opinions. – Flydog57 Dec 03 '18 at 21:56
  • 2
    If this is just for debugging, `ToString()` is preferred. Don't forget about [DebuggerDisplay](https://stackoverflow.com/questions/6829032/how-to-get-tostring-to-show-up-in-debug) – John Wu Dec 03 '18 at 21:58
  • I got it, no opinions is a must. But I think we can say something about objective aspects such as readability and/or maintainability, can't we? – Ricardo Fuente Dec 03 '18 at 22:01
  • I submitted an edit to make it less opinion-centric. Hopefully it'll get accepted by the mods. Because, to be honest, it's a good question. – Kevin Dec 03 '18 at 22:01
  • 1
    @RicardoFuente Readability and maintainability aren't objective, they're extremely subjective and lots of people have different *opinions* on what is "readable" and what isn't. – Servy Dec 03 '18 at 22:03
  • 3
    We can also say objective things about the by-design purpose of a thing. The by-design purpose of `ToString` is to give a friendly, human-readable summary of the value of an object for display or debugging purposes. Are you intending to give a friendly, human-readable summary of the value of an object for display or debugging purposes? If yes, then why would you do something *other* than `ToString`? – Eric Lippert Dec 03 '18 at 22:04
  • That said, this is a perfectly reasonable question and it should not be closed or deleted. – Eric Lippert Dec 03 '18 at 22:10
  • The question "what are the Microsoft-blessed best practices encouraged by the designers of this method?" is also an objective question. If you have not already, you should read the "best practices" section of https://learn.microsoft.com/en-us/dotnet/api/system.object.tostring – Eric Lippert Dec 03 '18 at 22:19
  • I tried editing, but it was rejected as 'superfluous' - that it didn't clarify or improve readability (even though it wasn't attempting to make it more readable). I just wish the last paragraph simply asked if "there are objective reasons to prefer any of the options, or if it's simply a matter of personal preference." Bam - it's no longer opinion-based, and is a good question for the site. – Kevin Dec 03 '18 at 22:20
  • @Kevin Include more changes, such as removing the c# from the title since it's a tag. – LarsTech Dec 03 '18 at 22:23
  • Will attempt another edit. But... it seems weird to reject an edit if I didn't find *all* the things to fix (instead of just the big thing that's putting the question on hold.) – Kevin Dec 03 '18 at 22:25
  • @Kevin The title is problematic though too. – LarsTech Dec 03 '18 at 22:26

3 Answers3

1

For debugging purposes, ToString() wins hands down.

Why? Because when you're stepping through code using VisualStudio, VS will easily display the ToString() results when you hover over a variable, or put the variable in the watch window. Otherwise, you have to dig in to get the property you're concerned with. This can be especially annoying if you're working with lists/enumerations/etc.

Also, ToString() already exists, and is already supposed to be the text representation of an instance of your object. That's the whole point of it. Why add another property that's also the string representation of your object?

Kevin
  • 2,133
  • 1
  • 9
  • 21
  • I'm kinda curious about the downvote - I think my answer's pretty straight-forward, concise, and factual. Am I missing something here? – Kevin Dec 03 '18 at 22:05
  • It is a mystery. This answer looks concise and factual to me. – Eric Lippert Dec 03 '18 at 22:06
  • In my Opinion, `ToString()` should be able to produce a serializable version of the underlaying object. Which means the string needs to contain everything required to deserialize it into the object again. Therefore, `ToString()` should not be abused for some short-cut debug outputs of only a few properties of an object. (i.e. firstname / lastname of User) For that, use a custom property/method `DisplayName()` or something like that. – dognose Dec 03 '18 at 22:10
  • @dognose ToString is a function, not a property. – LarsTech Dec 03 '18 at 22:11
  • 1
    @dognose: Your opinion is not shared by the designers of the runtime. They note that **if the object is deserializable from a string, then you should consider making ToString produce the serializable form**. That is a far weaker position than the strong position you are taking. They note that rather, the by-design purpose of ToString is to make a **short, friendly, human-readable, debugger-compatible summary of the state of an object**. – Eric Lippert Dec 03 '18 at 22:12
  • 1
    @dognose, That's not what ToString() is meant for. There's a whole separate way of doing serialization in .net - you're not supposed to build your own, using the ToString() as a makeshift 'Serialize' function. Plus, well, if you're debugging something in VS, the last thing you want to do is serialize a massive instance whenever you try to hover over a variable. – Kevin Dec 03 '18 at 22:13
  • (That last part is *very* important. ToString() is supposed to be a quick, low-cost function. Serializing a massive data object might be a time-consuming task.) – Kevin Dec 03 '18 at 22:14
  • @EricLippert True, but there is no reason as well, why one would violate that "advice" even for non-deserializable objects? Worst thing is inconsistency within the same application. Imagine: "Oh yeah, that one can be searialized by `ToString()` - No, here you need to use `ToSearializedString()`, cause we use `ToString()` for Textboxes only" – dognose Dec 03 '18 at 22:15
  • @dognose, now, do not fall into the pernicious trap of saying "My object is serializable as JSON, JSON is serializable as a string, therefore `ToString` should produce the JSON serialization." That is not good logic and not good object design. Rather, the object should implement serialization logic that traffics **in json objects**, and if the user wishes to convert those to or from strings, the user can do so. – Eric Lippert Dec 03 '18 at 22:16
  • 1
    @dognose: You should not use `ToString` for serialization to begin with. The advice is not "use ToString for serialization". Use serialization methods for serialization. The advice is **if** an object has a serialized string form, then **consider** making ToString produce that form. That consideration involves good judgment, and judgment is the art of making compromises when faced with conflicting goals. – Eric Lippert Dec 03 '18 at 22:18
  • @dognose In the scenario you described, `ToSerializedString()` would always be used return the serialized object (and do you mean json serialized, xml serialized, or something else, by the way?). [`ToString`](https://learn.microsoft.com/en-us/dotnet/api/system.object.tostring?view=netframework-4.7.2) is for **display** purposes. If serialization was the main purpose, then wouldn't the default implementation use reflection to build a serialized representation of the type rather than just returning the fully-qualified **name** of the object's type? – Rufus L Dec 03 '18 at 23:59
1

I advise you to make property private and use it internally as value for attribute [DebuggerDisplayAttribute] like this

[DebuggerDisplay("{ReadableValue},nq")] public class DoubleParameter { private string ReadableValue { get; } }

Nick Reshetinsky
  • 447
  • 2
  • 13
0

Thank you very much for all your comments.

I am in the line of most of you: use ToString() and it's good to know that I'm not alone. But in the end, it seems we are going to 'save' it for later logging purposes.

@LarsTech: Thanks for the edition, much clearer ;)

Ricardo Fuente
  • 33
  • 1