2

I have a class:

    public class Person
{
    public string FirstName { get; private set; }
    public string LastName { get; private set; }
    public string Email { get; private set; }
    public string Telephone { get; private set; }
    public Address Address { get; private set; }

    public Person(string firstName, string lastName)
    {
        //do null-checks
        FirstName = firstName;
        LastName = lastName;
        Address = new Address();
    }

    public void AddOrChangeEmail(string email)
    {
        //Check if e-mail is a valid e-mail here
        Email = email;
    }

    public void AddOrChangeTelephone(string telephone)
    {
        //Check if thelephone has correct format and valid symbols
        Telephone = telephone;
    }

    public void AddOrChangeAdress(Address address)
    {
        Address = address;
    }

The properties that are not in the constructor are optional, i.e. the person does not need an e-mail, address or telephone. However, I want to give the user of the class an opportunity to create the object without having to first provide the required information and then afterwards have to find out what methods to use to add the information.

Questions:

  1. Is it right to create 3 additional overloads to give them that option?
  2. Should i allow public setters on the optional properties and do the validation there?
  3. If the person marries and changes last name, do I need additional method to change the last name or should I make this setter public too, and just require them in constructor?
cfs
  • 1,304
  • 12
  • 30
  • 3
    Email and Telephone should be value objects like Address is. They will do their own validation. The Person shouldn't care about validating them, a valid email or telephone are objects that can be used by other objects and they should always be in a valid state. – MikeSW Oct 28 '12 at 08:53
  • @MikeSW Thanks. You're probably right and that would be an elegant solution to use more value types for fields like that so it can be used over and over again. – cfs Nov 02 '12 at 23:10

3 Answers3

4
  1. No.
  2. Yes
  3. Make it public.

Assuming you are going to have more code in the AddOrChange methods such as formatting logic or validation then I'd do the following. Otherwise, I'd completely get rid of the AddOrChange methods:

public class Person
{
    private string _email = string.empty;
    private string _telephone = string.empty;
    private Address _address = new Address();

    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string Email { 
        get { return _email }
        set { AddOrChangeEmail(value); }
    }
    public string Telephone { 
        get { return _telephone;}
        set { AddOrChangeTelephone(value); }
     }
    public Address Address { 
        get { return _address;  }
        set { AddOrChangeAddress(value); }
    }

    public Person(string firstName, string lastName)
    {
        //do null-checks
        FirstName = firstName;
        LastName = lastName;
    }

    private void AddOrChangeEmail(string email)
    {
        //Check if e-mail is a valid e-mail here
        _email = email;
    }

    private void AddOrChangeTelephone(string telephone)
    {
        //Check if thelephone has correct format and valid symbols
        _telephone = telephone;
    }

    private void AddOrChangeAddress(Address address)
    {
        _address = address;
    }
}

To work with this class you could do any of the following:

Person p = new Person("Tom", "Jones");
p.Telephone = "9995551111";

or

Person p = new Person("Tom", "Jones") { Telephone = "9995551111", Email="spamme@ms.com" }
NotMe
  • 87,343
  • 27
  • 171
  • 245
  • your syntax for properies is a bit off; you'll need to create a backing field and manually set/return it from the properties. – Servy Oct 19 '12 at 16:37
  • @Servy: quite true. That's what I get for copy/paste and typing fast.Fixed. – NotMe Oct 19 '12 at 16:38
3

AddOrChange is equivalent to simple property with public setter, thus you don't need those methods.

public class Person
{
    public string FirstName { get; private set; }
    public string LastName { get; private set; }
    public Email Email { get; set; }
    public Telephone Telephone { get; set; }
    public Address Address { get; set; }

    public Person(string firstName, string lastName)
    {
        //do null-checks
        FirstName = firstName;
        LastName = lastName;
    }
}
  1. If user during person creation want to provide something additional beside required data, she can use class initializers. Also you can add some optional parameters to constructor.

    var bob = new Person("Bob", "Uncle") { Address = someAddress };

  2. If its OK for person to relocate, then why not to use public setter for changing address? Of course, you should check if address is valid. Also if relocating is a business process (i.e. you are relocating someone in hotel) then it would be nice to have this operation on domain service (which will verify if destination room is empty and ready).

  3. Allowing to change name is OK. Usually name is not identity for such entities, thus it could change.

Also, I'd introduced value objects for email and telephone. I think it is not person's responsibility to verify if email address valid. Move that to Email class. Same with Phone and Address.

Sergey Berezovskiy
  • 232,247
  • 41
  • 429
  • 459
  • do not agree at all... properties does not carry semantics in the same way – Roger Johansson Oct 19 '12 at 16:28
  • my 2 cents : http://stackoverflow.com/questions/4451689/domain-driven-design-domain-objects-attitude-about-setters/4497283#4497283 – Roger Johansson Oct 19 '12 at 16:31
  • 1
    Setter says, that some dependency or data can be changed. If you want to give name for changing process - it's up to you. E.g. use method `Relocate` instead of setter for `Address`. But `AddOrChange` brings nothing new, beside what we already know. – Sergey Berezovskiy Oct 19 '12 at 16:35
  • that is just bad method names, I still beleive there should be command methods for this, just with other names.. names of verbs in the domain language at hand.. otherwise it is not very DDD at all – Roger Johansson Oct 19 '12 at 16:37
  • @RogerAlsing you really think properties should not be used in DDD application? – Sergey Berezovskiy Oct 19 '12 at 16:40
  • Two good answers to this question. The reason for asking was the different perspective like the discussions here. I got convincing arguments for having all setters private, but when I realized the consequence in more basic entities like this it seemed like duplicating code that is already there. I can agree that method Relocate could be used. – cfs Oct 19 '12 at 16:54
  • preferably not, if the data can be altered, there really should be some domain verb associated with that change. – Roger Johansson Oct 19 '12 at 16:55
1

Is lots of add/change methods and constructor overloads a consequence of DDD?

No, lots of updating methods is not consequence of DDD.

Code

Your Person class can be rewritten to have only 2 updating methods:

class Person

    public function Rename(FirstName as Name, LastName as Name) as Person

    public function ChangeContacts(
        Address as Maybe(of Address), 
        Phone as Maybe(of Phone), 
        Mail as Maybe(of MailAddress)) as Person
end class

Rename method accepts two required parameters of special Name type. Validation checks for names happen when names are created, not when they are passed into Person class.

ChangeContacts method accepts three optional parameters any of which can be absent. Special Maybe type indicates that they are optional. Special Address, Phone and MailAddress types indicate that these parameters are already valid and there is no need to validate them again in Person class.

Use case

Person marries and changes last name

Person = Person.
    Rename(Person.FirstName, LastNameAfterMarriage)

Person buys new phone number

Person = Person.
    ChangeContacts(Person.Address, NewPhoneNumber, Person.Mail)

Person lost phone number

Dim NewPhoneNumber = Maybe.Create(Nothing)
Person = Person.
    ChangeContacts(Person.Address, NewPhoneNumber, Person.Mail)

The pattern is to call updating method with old values + some new values.

Lightman
  • 1,078
  • 11
  • 22