Search code examples
c#classfieldencapsulationaccessor

Is it bad practice to use accessors from within a class?


So I have a simple class, User, which is something like this (ignore horrible whitespace use, wanted to keep it brief to read online):

public class User
{
  private string username;
  public string Username 
  {
    get
    {
      return username;
    }set{
      if(Validate.usernameIsValid(value)){username = value;}
      else{throw new InvalidArgumentException("Username is invalid");}
    }
  }
  //some more fields

  public User(String argUsername)
  {
    this.Username = argUsername;
    //OR validate again before:
    username = argUsername;
  }

}

Is it better to use the public accessor within the class to use its validation? Or is that bad practice and in that case, should I re-validate before setting the private username field?


Solution

  • I'd recommend using the public setter over local setting of the variable, simply because there'll be one place - the setter - where all the logic related to validation is handled. But this is only effective if you follow this convention every where within the class and all the derived versions strictly.

    One cannot assume that a member variable is not manipulated elsewhere in the class ( or its derived versions, if it is protected). Imagine another programmer debugging an error related to username validation. It can be a pleasant surprise to find out upon search, that all validations take place via the setter - so she doesn't haven't to debug multiple validation logic.