8

So I got the Address class:

class Address 
{
    private String streetAddress;
    private int number;
    private String postalCode;
    private City city;
    private State state;
    private Country country;
}

And I want to get its readable version to, lets say, show in a grid column.

Whats the best and concise way to implement this?

  1. toString method inside class Address (I personally don't like this approach, as 'toString' is not directly related to an Address)
  2. class ReadableAddressFormatter
    • ReadableAddressFormatter(Address addressToFormat)
    • public String getFormatted()
  3. Previous class but getFormmated would be static, receiving the Address instance and returning the string
  4. Other? Suggestions please.

I'm looking for a good design, focusing also in Clean Code, Decoupling and Maintainability.

Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321
Yuri Ghensev
  • 2,507
  • 4
  • 28
  • 45
  • 3
    Option 2 is what I would do as it decouples the model from the presentation concerns. – Deleted Oct 26 '11 at 18:08
  • 3
    I beg to differ with your statement that "'toString' is not directly related to an Address". Printing an address is central to an address' purpose. – Chris Pfohl Oct 26 '11 at 18:11
  • 3
    @Cpfohl: ToString is used to return a string representation of an object. It is NOT a presentation tool. It's an infrastructure concern. – Deleted Oct 26 '11 at 18:21
  • 1
    Your targeting of multiple languages is bound to cause differing opinions for option 1, as it appears to be an accepted approach in c# to use the toString for end user display proposes, while it is frowned upon in Java except for very simple data types. Yours is not a simple type. – Robin Oct 26 '11 at 18:24
  • @ChrisSmith Everything about this question smells of Java, yet it is brazenly tagged with C#. – Grant Thomas Oct 26 '11 at 18:30
  • @Mr Disappointment: Well the principle applies across both languages be it toString() or ToString() – Deleted Oct 26 '11 at 18:31
  • @Mr.Disappointment developing both in Java and C#, I see no reason to think different when designing systems with one of them. The same is valid for coding/naming style. – Yuri Ghensev Oct 26 '11 at 18:32
  • 1
    @ChrisSmith No, it [isn't](http://msdn.microsoft.com/en-us/library/system.object.tostring(v=VS.100).aspx): "ToString is the major formatting method in the .NET Framework. It converts an object to its string representation so that it is suitable for display." – Grant Thomas Oct 26 '11 at 18:34
  • 5
    @Mr.Disappointment: It's used trivially (logging, simple structs, basic types etc) but for proper presentation concerns, you should have some kind of separate transformation/conversion implementation as per WPF and MVC. The "Address" above is an unsealed complex type which may have several presentation modes so you are violating a) the SRP and b) Liskov substitution rules if you use ToString. Also what about internationalization. SRP will be royally smashed if you have to include several international address formats. – Deleted Oct 26 '11 at 18:40
  • 1
    @ChrisSmith You can have "separate" implementations, using format providers, which can support "several presentation modes" and internationalisation, also maintaining responsibilities; none of this destroys substitution either. – Grant Thomas Oct 26 '11 at 18:49
  • @Mr.Disappointment: which is option 2 is it not (albeit genericised a little). – Deleted Oct 26 '11 at 19:06
  • 1
    @ChrisSmith Rather arguably a bespoke attempt at interoperable implementation, yes - but again, there is a concrete foundation to build on in .NET which this pays no attention to and illustratively defies such mechanisms, so the "correct" answer _would_ differ in specifics between the two languages. But, so as not to be a pedant, I can stop here and simply beg to differ. – Grant Thomas Oct 26 '11 at 19:13

7 Answers7

7

All of these methods have been used, and there's no way to offer a "context independent" best practice. The best answer in Software Engineering is usually "it depends." That's said, let's analyze each:

  1. The KISS approach at its finest. I do this for all my basic "print to console, make sure things are working" kind of thing. If you have a specific format you can expect for addresses, this is the low-hanging fruit/easy win solution. You can always override this or print out the object differently in one off situations.
  2. This is the most extensible solution, in that it will nicely allow for localization and custom formatting. Whether it is appropriate depends on how often you expect addresses to be shown in different formats. Do you really need that Death Star to knock out a fly, or is the ability to change to all uppercase or change between languages pivotal to your app?
  3. I would not suggest this approach, as it generally started to bleed "view level" logic into the Domain, which is usually best handled by other tiers (in a class MVC approach). One could argue that toString() does the same thing, but toString() can also be thought of as the "name" or "essence" of how an object appears to the external world, so I'd say it's more than just presentational.

Hope this helps, and kudos for thinking about Clean Code, Decoupling, and Maintainability from the beginning.

For an example of principle #2 in action--using the Strategy Pattern, adhering to the Single Responsibility Principle, the Open/Closed Principle and allowing for Inversion of Control via Dependency Injection-- compare the following approach (graciously provided by @SteveJ):

public class Address {
        private String streetAddress;
        private int number;
        private String postalCode;
        private String city;
        private String state;
        private String country;

        public String toLongFormat(){
            return null; // stitch together your long format
        }

        public String toShortFormat(){
            return null; // stitch together your short format
        }

        public String toMailingLabelFormat(){
            return null; // stitch together your mailing label format
        }

        @Override
        public String toString(){
            return toShortFormat(); // your default format
        }
    }

}

With this one (in "mostly correct" Groovy):

public interface AddressFormatter {
   String format(Address toFormat)
}

public class LongAddressFormatter implements AddressFormatter {
    @Override
    public String format(Address toFormat){
         return String.format("%sBLAHBLAHBLAHBLAHBLAHBLAHBLAHBLAHBLAHBLAHBLAHBLAHBLAHBLAHBLAHBLAHBLAH%n%s", toFormat.streetAddress, toFormat.postalCode)
    }
}


public class ShortAddressFormatter implements AddressFormatter {
    @Override
    public String format(Address toFormat){
         return String.format("%d", toFormat.number)
    }
}

public  class Address {
        private String streetAddress;
        private int number;
        private String postalCode;
        private String city;
        private String state;
        private String country;
        public  AddressFormatter formatter = new ShortAddressFormatter(); // just to avoid NPE

        public void setFormatter(AddressFormatter fr) { this.formatter = fr; }



        @Override
        public String toString(){
            return formatter.format(this); // your default format
        }
    }

def addrr = new Address(streetAddress:"1234 fun drive", postalCode:"11223", number:1)
addr.setFormatter(new LongAddressFormatter());
println "The address is ${addrr}"
addr.setFormatter(new ShortAddressFormatter());
println "The address is ${addrr}"

As @SteveJ has observed:

" So the you have different formatting "strategies" and you can switch between them...I had this idea that you would set the formatting once and be stuck with it...AND if you want to add another formatting style, you don't have to open up and rewrite the address class, but write a new separate style and inject it when you want to use it."

  • 2
    You may have a race condition with `setFormatter()`. I'd suggest passing the `Address` into an `AddressFormatter` in the style of `Date` and `DateFormat` something like `AddressFormatter.format(Address)`. – Mike Christianson Nov 03 '11 at 05:20
6

.NET SOLUTION:

Overriding Object.ToString() seems to be the most logical solution. This makes it clean to use in situations such as: Console.WriteLine("Home Address: {0}", homeAddress);

If you wish to provide additional formatting, the Address class should implement IFormattable.

Also, you should create an AddressFormatter class that implements from IFormatProvider and ICustomFormatter.

The MSDN links provide very well put examples (a BinaryFormatter and a AcctNumberFormat), but if those aren't enough also look at this good example: PhoneFormatter


Additionally, if you do decide to go full out on this and implement IFormattable and a custom IFormatProvider/ICustomFormatter then I'd suggest having your ToString() simply call to your ToString(String format, IFormatProvider formatProvider) with a default provider. That way you can account for things like localization and types of addresses (short, long, etc).

myermian
  • 31,823
  • 24
  • 123
  • 215
  • Steve J: I don't follow, why would that matter when this is a C# question? – myermian Oct 26 '11 at 18:28
  • 1
    Actually he tagged it as C# and Java, I guess I should mark my answer as .NET specific, but I'm sure Java has some similar way of formatting strings implementing interfaces. – myermian Oct 26 '11 at 18:37
2

Using toString requires no additional baggage outside the function itself; seems like the simplest solution. It's there for a reason, right?

Toomai
  • 3,974
  • 1
  • 20
  • 22
  • 5
    It's not there for this reason. What if the country is internationalized? What if the parts of the address aren't displayed in the same order depending on the locale? What if it needs to be displayed as text sometimes, and as HTML other times? – JB Nizet Oct 26 '11 at 18:08
  • Then perhaps a superclass would be appropriate that overrides the toString() method. – williamg Oct 26 '11 at 18:13
  • It would depend on the context. If the application's scope doesn't reach into those areas then there's not too much reason to cater for them - unless you already know it's a future possibility. – Toomai Oct 26 '11 at 18:17
  • @JBNizet: For most classes, the ToString() method looks at the current internationalization settings to determine what to emit. This is an entirely valid approach. – NotMe Oct 31 '11 at 19:09
2

Usually i divide presentation layer from data layer. Presenting it to a GUI seems to me something related to the presentation layer, not the data layer.

I would suggest you to put a function somewhere in your presentation layer that will convert the address to string.

Presentation of data is not related to data!

A static method is good. A converter class would be better, you can keep one single instance for your application but you can replace it or write another if you are moving your application from GUI to WEB with another format, or if in one window you want to show everything and in another window you want to show only part of the informations or informations formatted in another way.

There are several model you can follow, for example Microsoft WPF uses totally another approach, the MVVM, Model View View Model, that will allow you to divide very well data layer from business logic from presentation layer.

I usually override ToString in C# or toString in java only for debugging purposes (presenting a string that i can use for debug) or for some kind of simple serialization to string, usually putting also a FromString (or fromString method in java). One example is custom types like Point, Vector, Matrix and so on.

Talking about C# world..

public class AddressToStringConverter
{
    public virtual string ToString(Address address)
    {
        return address.Street + ", " + address.City
    }
}

Then in your form (for example).

AddressToStringConverter myConverter = new AddressToStringConverter();

public Address CurrentSelectedAddress { get { ... } }

public button1_click(object sender, EventArgs e)
{
    button1.Text = myConverter.Convert(address);
}

If you want you can imlpement other useful interfaces like for example ITypeConverter

Salvatore Previti
  • 8,956
  • 31
  • 37
  • Just an FYI, there is a better alternative to enhance the debugger information on a class than to use the `ToString()` method, instead use the `DebuggerDisplayAttribute` because that is exactly what it was meant for: https://msdn.microsoft.com/en-us/library/ms228992(v=vs.110).aspx – myermian Oct 04 '16 at 20:35
1

toString() is the most flexible and convenient, being implicitly called when you combine an object of the Address class with a String, as in System.out.println("My address is " + objectOfAddressClass).

The only reason I can think of to not override toString() is if you need to alter the formatting. Then you would need different methods (as in toMailingString() and toShortFormString() and so on) or a parameterized method (as in toMailingString(boolean useShortForm) or whatever), but either way, toString() won't cut it.

Of course, you can (and should) do both. Have toString() as your default, probably calling one of your specific format methods, and then have your other helper methods for alternate formats.

public class TestClass {

    class City{};

    class State{};

    class Country{};

    class Address {
        private String streetAddress;
        private int number;
        private String postalCode;
        private City city;
        private State state;
        private Country country;

        public String toLongFormat(){
            return null; // stitch together your long format
        }

        public String toShortFormat(){
            return null; // stitch together your short format
        }

        public String toMailingLabelFormat(){
            return null; // stitch together your mailing label format
        }

        @Override
        public String toString(){
            return toShortFormat(); // your default format
        }
    }

}
Steve J
  • 2,676
  • 20
  • 18
  • Too much view logic in your class in this approach. Look at all those methods! It would quickly become unwieldy. Single Responsibility Principle says an injected Formatter that's delegated to would likely be architecturally superior. – Visionary Software Solutions Oct 26 '11 at 18:35
  • 1
    I would say that you should never use toString() when you need to rely on the format. Which makes its usage outside of debugging and logging dubious at best. The only exception to this would be for very simple types with a single accepted display format. – Robin Oct 26 '11 at 18:41
  • Not sure what you mean by "too much logic". I have a class for which alternate formats are useful, and a method for each. One of those formats as my default format used to implement a meaningful version of toString(). There are all sorts of ways to skin this cat, like parameterizing a single formatting method, but then you trade three methods for one method and one enumeration with 3 members -- seems like more work, not less. But I will study this Single Responsibility Principle. I want to see how more wieldy it would be ("wieldy" being the opposite of unwieldy, of course). – Steve J Oct 26 '11 at 19:06
  • OK, I looked up Single Responsibility Principle. Meh. Seems obvious. This class has the responsibility of handling Addresses. Formatting an address is part of handling an Address. Not sure what pulling the formatting out so I can inject it back in does for me. I suppose if there were many different formats, sure. But for something as straightforward and commonplace as an address, it seems like using a sledge hammer to kill a fly. – Steve J Oct 26 '11 at 19:11
  • 2
    You've just shown skeletons here. Obviously the implementations that "stitch together" will have logic/processing inside of them, liking causing great expansion to the line number count and leading to an unreadable mess. Salvatore's bit works well here: "Presentation of data is not related to data!" We can argue that all address related functions should be in Address, "since that is its job", but that goes against the spirit of Clean Code. Robert Martin does a great job illuminating these concepts in his book. I encourage you to give it a read. – Visionary Software Solutions Oct 26 '11 at 19:45
  • 1
    @VisionarySoftwareSolutions that book inspired my question – Yuri Ghensev Oct 26 '11 at 19:50
  • @arkilus I could tell. It's pretty fantastic. You may enjoy other related musings: http://t.co/LDvsqi0i – Visionary Software Solutions Oct 26 '11 at 19:52
  • @VSS Just for giggles, maybe you can post what it should look like. – Steve J Oct 26 '11 at 19:57
  • Use the Strategy Pattern and leave the class Closed for Modification but Open for Extension (Open/Closed Principle) via Dependency Injection, Luke. https://gist.github.com/1323904 – Visionary Software Solutions Oct 29 '11 at 00:20
  • OK, I'm getting this. Thanks. So the you have different formatting "strategies" and you can switch between them. That was the part that was eluding me -- I had this idea that you would set the formatting once and be stuck with it. What you're showing makes a lot of sense. – Steve J Oct 29 '11 at 13:12
  • You got it! Glad to be of service. :) – Visionary Software Solutions Oct 30 '11 at 21:24
  • 1
    AND if you want to add another formatting style, you don't have to open up and rewrite the address class, but write a new separate style and inject it when you want to use it. Darn, this make sense. – Steve J Oct 30 '11 at 23:30
  • @VisionarySoftwareSolutions You could add your example to your answer up there – Yuri Ghensev Oct 31 '11 at 17:42
0

I think that a toString() method that returns a string is your best approach. If you have an Address instance, let's say address, then it is obvious what address.toString() does. The fact that toString() isn't directly associated with Address doesn't really change anything.

williamg
  • 2,738
  • 6
  • 34
  • 48
0

You've tagged your post with Java, so I'll answer for Java (and Swing, more specifically). This task is normally the task of a specific TableCellRenderer. If the same format must be used for other visual components, I would indeed extract the formatting inside an instantiatable class (solution 2). This would allow subclasses to customize the format if needed.

JB Nizet
  • 678,734
  • 91
  • 1,224
  • 1,255