Validating an email address error

[saldous]

I can't work out what i've done wrong here, why is it telling me there is unreachable code on the "if" in this:

//Check email address entered is valid    
            public class RegexUtilities
            {
            public static bool IsValidEmail(string sEmailRecipient)
                {
                // Return true if sEmailRecipient is in valid e-mail format.
                return Regex.IsMatch(sEmailRecipient, 
                    @"^([0-9a-zA-Z]([-\.\w]*[0-9a-zA-Z])*@([0-9a-zA-Z][-\w]*[0-9a-zA-Z]\.)+[a-zA-Z]{2,9})$"); 
                
                if (RegexUtilities.IsValidEmail(sEmailRecipient))
                MessageBox.Show("Valid = " + sEmailRecipient);
                          
                else
                MessageBox.Show("Inalid = " + sEmailRecipient);
                   }
                
            }

Any ideas?

Thank you.

[Kheun]

Well, in your first code within IsValidEmail(), it always return the boolean result from Regex.IsMatch().

[boudino]

Maybe you've wish to write it this way:

public static bool IsValidEmail(string sEmailRecipient)
                {
                // Return true if sEmailRecipient is in valid e-mail format.
                bool isMatch = Regex.IsMatch(sEmailRecipient, 
                    @"^([0-9a-zA-Z]([-\.\w]*[0-9a-zA-Z])*@([0-9a-zA-Z][-\w]*[0-9a-zA-Z]\.)+[a-zA-Z]{2,9})$"); 
                
                if (isMatch)
                MessageBox.Show("Valid = " + sEmailRecipient);
                          
                else
                MessageBox.Show("Inalid = " + sEmailRecipient);
                   }
                
            }

Note that you had recursion in your code, which will lead to StackOverflowException.

[saldous]

Thanks for your help all, but I'm still struggling.

with the code above (from boudino) I get the error:

"not all code paths return a value"

I'm starting to hate c# already, I've been playing with this same bit of code all night now

[sotoasty]

It's not C#. It is basic programming. All functions must return a value to the calling procedure. This would happen in any language.
I am sure in re-arranging your code, buodino just missed adding in one line. Add the return statement after you if statement like below.

public static bool IsValidEmail(string sEmailRecipient)
                {
                // Return true if sEmailRecipient is in valid e-mail format.
                bool isMatch = Regex.IsMatch(sEmailRecipient, 
                    @"^([0-9a-zA-Z]([-\.\w]*[0-9a-zA-Z])*@([0-9a-zA-Z][-\w]*[0-9a-zA-Z]\.)+[a-zA-Z]{2,9})$"); 
                
                if (isMatch)
                MessageBox.Show("Valid = " + sEmailRecipient);
                          
                else
                MessageBox.Show("Inalid = " + sEmailRecipient);
             return isMatch;     
            }

Or better yet, consolidate the code something like.

public static bool IsValidEmail(string sEmailRecipient)
                {
                // Return true if sEmailRecipient is in valid e-mail format.
                if (Regex.IsMatch(sEmailRecipient, 
                    @"^([0-9a-zA-Z]([-\.\w]*[0-9a-zA-Z])*@([0-9a-zA-Z][-\w]*[0-9a-zA-Z]\.)+[a-zA-Z]{2,9})$"))
                       return true;
                else
                       return false;
            }

I say that because you probably don't really want to show a messagebox in that routine, you probably want to just show it if it is invalid.

[Arjay]

How about?

 
public static bool IsValidEmail(string sEmailRecipient)
{
  // Return true if sEmailRecipient is in valid e-mail format.
  return Regex.IsMatch(sEmailRecipient, Constants.RegExExp.Email );
}

[boudino]

Less code is better code

[saldous]

Thanks all, think I've got it sorted now. you've all been very helpful.