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 countriesCountryName
can be changed anytime, without any underlying logic or business rulesIsoCode
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?