0

I am just trying to learn c# and am I little confused about the best way to validate data that's input from a user.

I have this code in a Person Class to validate a phonenumber

public string PhoneNumber 
{
    get { return phoneNumber; }
    set
    {
        if (!Regex.IsMatch(value, @"^[0-9()+- ]+$"))
        {
            throw new ArgumentException("Error Phonenumber: must contain numbers 0-9 & characters ()+-");
        }

        phoneNumber = value;
    }
}

The problem is that when this is executed it stops the program, all I want it to do is tell the user that the input data is incorrect, stop the creation of the object and let the user correct their mistake and try again.

I have read lots of questions etc online that are similar but nothing is giving me a concrete answer to solve this problem.

Thanks in advance.

2

The first step is to validate the user's input before passing it to the constructor. That gives the most control over how validation failures are handled.

When you validate within the constructor and throw an exception on failure, the calling code should wrap the object construction in a try block and catch the exception. Scope issues make the code for this more intricate if complex failure handling's desired, it's most suited for use in conjunction with validation before use. Errors should then be caught by the validation and the code would never get to the point where it constructed the object, an exception out of the constructor would be treated as a catastrophic application logic failure. The code to simply make the application die is much simpler, and failures of the validation code resulting in an exception will be rare enough that it's acceptable.

  • Thanks, i think thats what I was missing a try catch around the object creation, is it still best practise to check validation before object creation? I have read conflicting things. – Michael Williams Jul 31 '16 at 4:57
  • It depends on the language and the speaker. Some advise doing all error handling via exceptions on the grounds that it simplifies the code. I find that sometimes it does, but sometimes as in this sort of case it simplifies part of the code by complicating other parts far more. I go with whichever approach results in the simplest, most straightforward code overall on the grounds that that kind of code's less likely to be a source of bugs or maintenance migraines. – Todd Knarr Jul 31 '16 at 5:32
  • Thanks for your help, I tried up voting your answer but that was my first question so I don't have the privileges yet. – Michael Williams Jul 31 '16 at 5:39
  • Actually, validation before creation is the best thing you can do. But why store phone numbers as a string? They have some complex requirements, so create a class PhoneNumber that validates as part of its constructor and expose a static IsValid method. A person can only take a PhoneNumber object, which enforces its own validity and puts the validation requirement not on the person but on a singly responsible object. – JDT Jul 31 '16 at 11:28
  • And for the love of <deity>, DO NOT use exceptions for control flow like this post suggests. Calling code should (almost) never wrap a constructor in a try/catch simply for the purpose of validation. Exceptions from a constructor are unrecoverable and should be treated as such, meaning the exception bubbles up and is handled by the generic error handling in your application. – JDT Jul 31 '16 at 11:32
1

I hate dealing with objects that can get into invalid states. If you can reasonably design an object so that it's state simply can't be made invalid please do so.

Doing so means you have to check everything that can change object state and somehow disallow the change. Either by throwing an exception or insisting that state not change. While I hate things that fail quietly sometimes they make sense. Pushing the up button when the elevator is on the top floor shouldn't shut down the elevator or put it on the roof. But in your case throwing an exception is entirely correct. At least as correct as your regex anyway.

0

One alternative to the try-throw-catch workflow - which has some issues with it being fairly complex to follow at times - is to use a more functional approach, where the possible "errors" are declared in the return type. Now, I know this is not always easy to emulate in C#, but one pretty clean solution is to make sort of a "factory" function for phone numbers that has the signature Either<PhoneNumberValidationError, PhoneNumber> CreatePhoneNumber(string number), and then you FORCE calling code to consider the failure case. The syntax doesn't have to get that messy either (you can basically recreate pattern matching using Func delegates, but I realize this is sort of fighting against the language), but it might not be 100 % idiomatic C# (yet ;)).

It's an interesting approach to consider though, and if nothing else I think it's one of all the arguments for looking into what F# can offer.

0

Throwing in the class you show is fine, even in the constructor. This is often frowned upon in C++ for various reasons, however, works well in C#.

However you will have to write additional code to bridge the gap between stopping the constructor and stopping the program: this bridge might inform the real user that the input was invalid, and resume to a previous state of the program, asking for input.

That gap is bridged as as @RobertHarvey mentions, with a catch, by some caller (or caller's caller).

However, you don't necessarily want the immediate caller to look for exceptions. If all immediate callers wrapped all calls in try/catch, we'd probably be better off without exceptions at all (using error code returns instead).

You should try to identify the places in the calling code where failure means you are able to somehow retry. There is a bit of an art to this, but should try to think in higher level terms about the job or part of the job that can be aborted when something goes wrong, and what you might want to resume or repeat in those cases. Sometimes, thinking about what the program should do next regardless of whether some job fails or succeeds will help tell you where to handle errors.

In your question you are already indicating the higher level task that you want to retry. Now, it may be that you already have a high level loop in your program so that when it gets good input and succeeds, it doesn't simply quit but ask the user for another input to work on. If you don't have such a loop then you can add one. That high level loop can be a good place to locate the catch. Every iteration of the loop either succeeds (and you inform the user and go again) or it fails, and on failure, you catch, and inform the user, then go again.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.