Best place to check for property values.

Question

Hi,

Assuming you have a class Photo:

class Photo
{
  public string Title {get; set;}
  public string FileExtension {get; set;}
  public void Save() 
  { 
    // Save to backing store here 
  }
}

Which would be the best place to check whether the values have been set correctly. In the propert setter or the Save method.

Edit: What if it is an Update method? If photos are added through a PhotoManager, using a method like PhotoManager.Add(Photo p), then which is the best place to do the validation.

Kind regards,

Answer 1

If those are the only options, then in the property setter. Separation of concerns dictates that Save should do the saving, and not anything else.

Also, that is the beauty of properties. They look like fields, but they can have enhanced logic behind the getting and setting of the value in the backing field.

Answer 2

It's hard to say with so little detail. If you think it is valid for an object to be in an invalid state whilst it is being updated then I would say you have to do your validation in the Save method.

Answer 3

If I understand your question correctly, you could do both simply with a private boolean Validate(title, fileExtension) method called from within the settor and storage methods. If this validation is false, you can throw an Exception and deal with it from there.

Answer 4

Don't let your instances fall into an invalid state: validate right in the setter, that's what it's there for.

Answer 5

public void Save()
{
  try
  {
    // saving
  }
  catch(Exception ex)
  {
    MessageBox(ex.Message); // roughly
  }
}

or

public void Save()
{
  if(!IsDataValid) // !(String.IsNullOrEmpty(this.Title) && String.IsNullOrEmpty(this.Extension))
  {
    throw new Exception(); //in fact you need to throw something more concrete
  }
  // saving
}

Also I could use:

public string Title { get; private set; }

public string Extension { get; private set; }

public Photo(string title, string extension)
{
  this.Title = title;
  this.Extension = extension;
}

Answer 6

It depends, when do you have enough data to check the correctness: If the correctness of FileExtension does not depend on the value of Title, and the correctness of Title does not depend on the value of FileExtension, you can check each of them inside the setters.

But if one of them depends on the other's value, and you don't know what is the order in which setters will be executed, you have to check Title and FileExtension only when you get them both. That can be in Save, or in some new Check method.

Also, maybe before doing Save you have to check, whether both Title and FileExtenstion were set.

So, to summarize, you may need the following checks:

• Check Title's correctness: in Title.set
• Check FileExtension's correctness: in FileExtension.set
• Check that Title.set and FileExtension.set were executed: in Save
• Check the "co-correcntess" of Title and FileExtesion: in Save

Regarding the Update method: I understand that you receive a Photo object with Title and FileExtenstion already set. In this case, you have to decide, whether you trust that Title and FileExtension were already checked in the Photo by whoever did set them. If you trust it, you don't have to change anything in my proposal above. But if you don't trust, then you also have to check Title's correctness and FileExtension's correctness in Save (or, again, in some new Check method).