2

I'm trying to port a years old MS Access app with spaghetti VBA code to C# and OOP, and I'm struggling to find the best way to put domain logic in my domain classes.

I'll use a Country class as a simple example. It has three properties with different business rules:

  • CountryCode can't be changed anymore once the country is created, because that would cause issues with a 3rd party app that uses the countries
  • CountryName can be changed anytime, without any underlying logic or business rules
  • IsoCode can be changed anytime, but must be exactly 2 characters long
    (IsoCode has actually more rules, but in this example let's just assume that "must be exactly 2 characters" is the only rule, for the sake of simplicity)

I created two slightly different versions of the class.
I'm quite inexperienced in object-oriented programming, so I need help deciding:

  • does it matter which approach I use?
  • does one of them (or both) have issues that I don't see?
  • is there a different way that's even better?

Both of my approaches look good to me now, but I don't know if maybe they will cause problems later (the app in question is ten years old, and will probably live on for a long time).


Version 1:

public class Country1
{
    public string CountryCode { get; private set; }
    public string CountryName { get; set; }
    public string IsoCode { get; private set; }

    public Country1(string countryCode, string countryName, string isoCode)
    {
        this.CountryCode = countryCode;
        this.CountryName = countryName;
        SetIsoCode(isoCode);
    }

    public void SetIsoCode(string isoCode)
    {
        if (isoCode.Length != 2)
        {
            throw new ArgumentException("must be exactly 2 characters!");
        }

        this.IsoCode = isoCode;
    }
}

Version 2:

public class Country2
{
    public Country2(string countryCode, string countryName, string isoCode)
    {
        this.countrycode = countryCode;
        this.CountryName = countryName;
        this.isocode = isoCode;
    }

    private readonly string countrycode;
    private string isocode;

    public string CountryCode
    {
        get { return this.countrycode; }
    }

    public string CountryName { get; set; }

    public string IsoCode
    {
        get { return this.isocode; }
        set
        {
            if (value.Length != 2)
            {
                throw new ArgumentException("must be exactly 2 characters!");
            }

            this.isocode = value;
        }
    }
}

Some more background about why I'm asking this and what I want to know:

I have read lots of different opinions about the "right OOP way".
Some say that you shouldn't expose getters and setters at all. I understand why this is a bad idea with setters, that's why CountryCode can only be set from the constructor.

Some say that instead of using any getters and setters, you should use GetXXX and SetXXX methods. I can see that this makes sense in some cases (for example, a SetXXX method with several parameters when you have several values that need to be set together).
But often there are simple values like the CountryName in my example, which is a "dumb" value without any logic. When I have ten of these things in one class, I don't want to create GetXXX and SetXXX methods for each of them.

Then there's stuff like the IsoCode, which has no connection to any of the other properties either (so no need to use a SetXXX method to set it together with another property). But it contains some validation, so I could either make a SetXXX method, or just do the validation (and throw an exception when something is wrong) in the setter.

Is throwing an exception even the best way to notify the caller of an error? Some say it's fine, and some say that you should throw exceptions only for "exceptional" cases.
IMO it's not really exceptional when someone enters an invalid ISO code, but how else should I get the information that an error occured (including a human readable error message!!) to the client? Is it better to use a response object with an error code and a ErrorMessage string property?

Community
  • 1
  • 1
Christian Specht
  • 35,843
  • 15
  • 128
  • 182

2 Answers2

2

Personally I don't think there is much difference between the property and method implementation. I have a preference for using properties when the values can be set independently. I use setter methods to force more than one value to be provided to your model at the same time when the values depend on each other in some way (e.g. you are setting integers x and y and y cannot be larger than x).

It makes a lot of sense to have simple validation logic (e.g. required fields, fields lengths) in your view-model because this allows you to tie the validation more closely to the user interface (e.g. you could highlight the field that was invalid after it had been filled in).

I'd strongly recommend this article by Eric Lippert about what kind of condition makes a good candidate for throwing/catching an exception:

http://blogs.msdn.com/b/ericlippert/archive/2008/09/10/vexing-exceptions.aspx

Andrew Skirrow
  • 3,402
  • 18
  • 41
  • Concerning validation: right now I don't even have a viewmodel (as I said at the beginning of my question: Access/VBA spaghetti code). The frontend will most likely be Winforms or WPF (not decided yet), but anyway: having the validation logic in the view model is nice, but having it in the domain classes is more important IMO. Do you have a good example how I can re-use the validation logic in the domain classes **and** in the viewmodel? – Christian Specht Feb 25 '13 at 19:51
  • _Do you have a good example how I can re-use the validation logic in the domain classes and in the viewmodel?_ What happens when you don't: A new requirement: "Display value X in this screen." No problem. We had domain class A that was the class for that screen. Domain Class B already did the complex calculation for X, displaying it on another screen. So All one had to do in A was instantiate B and call the calculation method. Well, you could not even instantiate B because it was so tightly coupled to it's webForm/screen and several other classes. It took 3 months to fix it. – radarbob Mar 24 '13 at 15:45
1

I tend to use properties for single value changes and SetXXX methods rarely when I have some rule that involve more than one value that must be set at a time too (just like Andy Skirrow said).

SetXXX methods for everything are a common "Javaish" convention (althrough its used in many languages that don't have setters and getters like C#).

People try to force some good practices they learned for some language into every language they can, even if it did not fit necessarily well or the language have better alternatives for that.

Try not to be an "OOP maniac". This will only cause you pain in the head. OOP is not a religion (and even religion should not have fanatics IMHO, but this is another story).

Back to the point

The two approaches makes no functional difference and will not cause any harm in the future. Take the way you fill its more readable and pleasant to code. This will count much more than "the correct OOP way", cause many people have its own definition for the "better way" of doing things.

Validation

If you are using some structural or architectural pattern like MVC, you can use Data Annotation Attributes to enforce validation and, depending on the framework, use it to enforce client side validation as well.

See @Andy Skirrows's answer for links.

Ricardo Souza
  • 16,030
  • 6
  • 37
  • 69