4

I would like to create my own custom Exception (for my own practice), I have Man class and i would like to check the name (so its not empty, null and only English chars. I'm not sure if I'm doing this right, 1.do i need to write the code that handles with the error (if occures) in the Custom Exception class? or in the Man's setter? 2. Where should i use the "throw new Exception" for best practice? 3. any comments\improvements about my code would be welcome.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace prog
{
    class Program
    {
        static void Main(string[] args)
        {

            try
            {
                Man p = new Man("Dan");

            }
            catch (Exception e)
            {
                throw new NameNotValidException(e.Message);
            }


        }
}

class Man
{
    private string name;

    public string Name
    {
        get { return name; }
        set
        {
            if (name == "" || name == null)
            {
                throw new NameNotValidException("error");
            }

            name = value;
        }
    }

    public Man(string name)
    {
        this.name = name;
    }

}

class NameNotValidException : Exception
{
    public NameNotValidException()
    {
        Console.WriteLine("Please Write a valid name!");
    }

    public NameNotValidException(string message)
        : base(message)
    {
    }

    public NameNotValidException(string message, Exception inner)
        : base(message, inner)
    {
    }
}

Thanks!

Coder123
  • 784
  • 2
  • 8
  • 29
  • 2
    In this case it is more appropriate to throw `ArgumentNullException` instead. Also you want to check `value`, not `name`. – Igor Mar 01 '17 at 16:50
  • for your example, your catch should handle the exception, not raise a new one. – Jonesopolis Mar 01 '17 at 16:51
  • 2
    It must be `this.Name` not `this.name` - otherwise you don't use the setter method. Also you should not catch an exception and create the same new. you can use just `throw;`to throw the same exception more up (you will see in log). – Matt Mar 01 '17 at 16:57

5 Answers5

4

If you want to create a custom exception just extend any of the exception classes, e.g.:

class MyCustomException : System.Exception
{}

and the you can do throw new MyCustomException();

dcg
  • 4,187
  • 1
  • 18
  • 32
4
  1. In this case it is more appropriate to throw ArgumentNullException instead. Which exception you end up using (your own or ArgumentNullException) does not matter and does not change the structure of the code below OR how you should handle an Exception.
  2. You want to check value, not name in the setter.
  3. Handle the exception at the calling code. If the calling code is not designed to handle the Exception then do not catch that Exception OR rethrow using throw to preserve the stack trace.
  4. Throw the exception at the location where the code fails due to... (invalid value in this case)
  5. Be careful with your getter/setter code, you were checking the wrong values and also bypassing the setter in the constructor in which case it would never throw an Exception to begin with.

Your Man class.

public class Man {
    public Man(string name)
    {
        // notice capital N for Name so it is set on the property, not the field
        // this will execute the setter for the Name property
        this.Name = name;
    }

    public Man(){} // optional, but do not include the parameterized constructor you had as it sets the private fields directly OR include additional validation

    private string name;
    public string Name
    {
        get { return name; }
        set
        {
            if (string.IsNullOrEmpty(value))
                throw new ArgumentNullException("Name cannot be null or empty");
            name = value;
        }
    }
}

Calling code which handles the exception.

try
{
    // use parameterized constructor
    Man p = new Man("Dan"); 

    // or use an initializer
    Man p = new Man{Name = "Dan"}; 

    // the above initializer is actually short for
    Man p = new Man(); 
    p.Name = "Dan"; 
}
catch (ArgumentException e)
{
    Console.WriteLine("Error occurred!! Do something...");
}
Igor
  • 60,821
  • 10
  • 100
  • 175
  • 1
    But the question is about his own practice for custom exceptions. – Matt Mar 01 '17 at 16:59
  • @Matt - Whether `ArgumentNullException` is replaced by a custom Exception is irrelevant, the code structure above would stay the exact same. – Igor Mar 01 '17 at 17:01
  • thanks! what is the difference between the way you initiated "Dan" and the way i did that? – Coder123 Mar 01 '17 at 17:20
  • @Coder123 - your overloaded constructor set the private field `name` directly so the `set`er on the property `Name` was never actually called. This forces you to call the public members on the type like `Name`. See updated code which shows what an object initializer actually is short for. – Igor Mar 01 '17 at 17:35
  • @Igor should i also add the if (string.IsNullOrEmpty(value)) check in the constructor? – Coder123 Mar 01 '17 at 17:52
  • 1
    @Coder123 - if you wanted to capture `Name` in the constructor and validate the incoming value you have 2 options. 1) Validate the parameter with that check in the constructor. Downside is you have more (double) code. 2) Assign the value to the `Name` property in the constructor and the `set` on the property will then validate and possibly throw a `Exception`. – Igor Mar 01 '17 at 17:55
2

When you throw an exception you're saying "Hey, something went wrong!", so the caller can then do something about that. The exception's responsibility is to say what exactly went wrong, not how to handle it. So you should remove the Console.WriteLine("Please Write a valid name!"); from the exception. Instead, put that in the code that is actually expecting that error - i.e. your Main method. static void Main(string[] args) {

        try
        {
            Man p = new Man("Dan");
        }
        catch (NameNotValidException e)
        {
            Console.WriteLine("Please Write a valid name! " + e.Message);
        }

Also note that I'm using NameNotValidException in the catch block, not Exception. As a general rule you should be as specific as possible in handling errors - which is why we create custom exceptions in the first place =). For example, let's say you add an Age property, which throws an AgeNotValidException. If you catch Exception e, you'll say "Please Write a valid name!" for every error, including invalid ages. By treating every exception type separately, you can handle each error differently.

About your "throw new Exception" question, you're doing it correctly: You should throw exceptions when you are unable to do something - in this case, you are unable to set the user's name because the given name is invalid. However, you should also try and be more specific with your error messages, to make errors easier to recover from: In your case, you could change it to something along the lines of throw new NameNotValidException("Name can't be empty");, so you can tell the user not only that the name is invalid, but also exactly why.

Bill
  • 400
  • 2
  • 6
0

if you want to change the message only you could use this : throw new Exception("File check failed!");

Ar Win
  • 106
  • 7
0

As you need to check several types of invalid input (not empty, null and only English chars), my advice is to create the custom exception with a property for invalid input type. The example is below:

class InvalidInputCustomException : Exception
{
    public string InputExceptionType { get; set; }

    public InvalidInputCustomException(string inputExceptionType)
    {
        InputExceptionType = inputExceptionType;
    }
}

Then you’ll need to create your class Man in which set accessor the input (in this code keyword value) will be checked and code lines - throw new InvalidInputCustomException .. - with corresponding input exception type in this custom exception constructor will be included. This class example is below:

class Man
{
    private string _name;

    public Man(string name)
    {
        this.Name = name;
    }
    public string Name
    {
        get { return _name; }
        set
        {
            if (value == null)
            {
                throw new InvalidInputCustomException("null is not valid for input.");
            }
            else if (value == string.Empty)
            {
                throw new InvalidInputCustomException("empty is not valid for input.");
            }
            else
            {
                foreach (char ch in value)
                {
                    if (!(ch >= 'A' && ch <= 'Z') && !(ch >= 'a' && ch <= 'z') &&
                        !(ch >= '0' && ch <= '9'))
                    {
                        throw new InvalidInputCustomException($"non English character {ch} is " +
                            $"not valid for input."); ;
                    }
                }
            }
            _name = value;
        }
    }
}

The thrown exception must be caught in the place where to initialized Man class object its property Name is attempted to set (as for example:

p.Name = inputString

or through this object constructor as in the code example below). The example of the Console application code is below:

class Program
{
    static void Main(string[] args)
    {

        Console.WriteLine("Enter name and press key Enter:");
        string inputString = Console.ReadLine();
        try
        {
            Man p = new Man(inputString);
            Console.WriteLine($"Entered name - {p.Name} - is valid.");
        }
        catch (InvalidInputCustomException ex)
        {
            Console.WriteLine($"Invalid input type - {ex.InputExceptionType}. Please enter valid name.");
        }
        catch (Exception ex)
        {

            Console.WriteLine("Unhandled exception " + ex.Message);
        }
        Console.WriteLine("Press any key to finish the program.");
        Console.ReadLine();
    }
}

The code example is more for custom exceptions understanding purposes. In real applications, you need to avoid exceptions throwing in situations related to user information entering - in this case, data validation tools must be used. During custom exception creation in a real application must be provided at least a parameterless constructor and a best practice is to add three additional constructors: with a string, with a string and an exception, for serialization.

Sharunas Bielskis
  • 1,033
  • 1
  • 16
  • 25