Throw an error when the user enter null or empty string

Question

I'm attempting to resolve the following exercise:

You need to create a class named Product that represents a product. The class has a single property named Name. Users of the Product class should be able to get and set the value of the Name property. However, any attempt to set the value of Name to an empty string or a null value should raise an exception. Also, users of the Product class should not be able to access any other data members of the Product class. How will you create such a class?

I have created the following code but for some reason it does not throw the exception when the string is invalid:

class Program
{
    static void Main(string[] args)
    {
        Product newProduct = new Product();
        Console.WriteLine("Enter Product name:");
        newProduct.Name = null; //Console.ReadLine();
        Console.WriteLine("Product name is : {0}", newProduct.Name);
        Console.ReadLine();
    }
}

class Product
{
    private string name;
    public string Name
    {
        get
        {
            return this.name;
        }
        set
        {
            if (Name != String.Empty || Name != null)
            {
                name = value;
            }
            else
            {
                throw new ArgumentException("Name cannot be null or empty string", "Name");
            }
        }
    }
}

Is the exception not thrown because I do not have try-catch statement? I was also wondering is it possible to have only catch statement without try statement?

Solution 1

Your if state is wrong. Let's do a truth table:

if (value != String.Empty || value != null)

---

Name = null   True Or False  = True 
Name = "name" True Or True = True
Name = ""     False Or True = True 

Your if statement is always true!

I would re-write it thus:

if (value == String.Empty || value == null)
{
     throw new ArgumentException("Name cannot be null or empty string", "Name");  
}
else
{
     name = value;
}

---

you could just change the Or to and AND but I think the above reads better (the below has an unnecessary double negative):

if (value != String.Empty && value != null)
{
   name = value;
}
else
{
    throw new ArgumentException("Name cannot be null or empty string", "value");
}

---

As Dmitry Bychenko says, I didn't notice you were not testing for value. In getters you should use the value property. Not the name of your property

---

The second parameter (again pointed out by Dmitry Bychenko) in your exception should be:

The name of the parameter that caused the current exception.

MSDN

which in your case is the string "value":

throw new ArgumentException("Name cannot be null or empty string", "value");

Solution 2

Use String.IsNullOrEmpty Method (String). Change your set like this:

set
{
      if (!string.IsNullOrEmpty(value))
      {
            name = value;
      }
      else
      {
            throw new ArgumentException("Name cannot be null or empty string", "value");
      }
}

Also you can use String.IsNullOrWhiteSpace Method (String) that Indicates whether a specified string is null, empty, or consists only of white-space characters.

Solution 3

If you want different exceptions on null and on empty string (often null means that something is totally wrong, when empty string is just a format error):

public string Name {
  get {
    return name;
  }
  set {
    if (null == value)
      throw new AgrumentNullException("value");
    else if (String.Equals(value, ""))
      throw new AgrumentException("Empty values are not allowed.", "value");

    name = value; 
  }
}

In case you don't want to distiguish them:

public string Name {
  get {
    return name;
  }
  set {
    if (String.IsNullOrEmpty(value))
      throw new AgrumentException("Null or empty values are not allowed.", "value");

    name = value; 
  }
}

Note, that in both cases it's value that you have to test, not a property Name. In your original code the name's (and so Name as well) initial value is null and you'll get exception whatever you try to set.