0

I need to validate a user input based on condition. i wrote a regular expression to do so, but it's failing not sure why. Can somebody point where i am making mistake?

Regex AccuracyCodeHexRegex = new Regex(@"^[PTQA]((0|8)[01234567]){2}$");

This is what i am trying to validate(If the string is a subset of these strings then it is valid):

Phh, Thh, Qhh, Ahh where 'h' is a hex digit in the set {00, 80, 01, 81, 02, 82, 03, 83, 04, 84, 05, 85, 06, 86, 07, 87}

Ex: P00 is valid P20 is not valid

15
  • 3
    Validate how? What makes a valid versus an invalid string? It is not clear from your question if the string you posted is valid or not. Can you post several examples of valid and invalid strings and explain the validation rules? Commented Dec 6, 2012 at 10:41
  • I have given the check in the question.Phh, Thh, Qhh, Ahh where 'h' is a hex digit in the set {00, 80, 01, 81, 02, 82, 03, 83, 04, 84, 05, 85, 06, 86, 07, 87} means valid Commented Dec 6, 2012 at 10:42
  • 1
    The Regex is fine, i think the way you are applying it may be wrong. More code please :) Commented Dec 6, 2012 at 10:47
  • 1
    P00 doesn't match what you describe. You say Phh where each h is two digits (from a set), but P00 is Ph where h is two digits. Commented Dec 6, 2012 at 10:50
  • 1
    @Charu - Why is P00 valid? It shouldn't be (if h is 00 and you have two hh - P0000 should be valid). Commented Dec 6, 2012 at 10:51

5 Answers 5

2

Your Regex ^[PTQA](?:(?:0|8)[01234567]){2}$

Applies to the following :

P8001
P8002
P0281
P8005

and so on, because you are repeating the number matches by {2}

To validate something like P81 / P05, you need to change that to {1}

Sign up to request clarification or add additional context in comments.

1 Comment

you don't nedd {1}you can simplue use ^[PTQA](?:(?:0|8)[0-7])$
2

I would write :

^[PTQA]((0|8)[0-7])$

you don't seem to need the {2} which validates strings like P0707

1 Comment

can be simplified to ^[PTQA](?:(?:0|8)[0-7])$
2

you can simplify your regex to ^[PTQA](?:(?:0|8)[0-7])$ which will do the trick for you

if you need speed regex aren't terribly fast and usually simple lookups on static values can be implemented with a switch-case. They aren't that nice when it comes to maintainability but if the values are fairly stable and only used in this one place that shouldn't be too much of a concern. If it is you can use a HashSet of all the valid values.

Using a HashSet:

var leading = new[]{'P','T','Q','A'};
var firstDigit = new []{'0','8'};
var lastDigit = new []{'0','1','2','3','4','5','6','7'};

var set = new HashSet<string>(from l in leading
                              from f in firstDigit
                              from lst in lastDigit
                              select l + f + lst);

public bool IsOk(string value){
   return set.Contains(value);
}

or using switch-case:

public bool IsOk(string value){
   if(value.length != 3) return false;
   switch(value[0]){
       case 'P':
       case 'T':
       case 'Q':
       case 'A':
          switch(value[1]){
               case '0':
               case '8':
                   switch(value[2]){
                        case '0':
                        case '1':
                        case '2':
                        case '3':
                        case '4':
                        case '5':
                        case '6':
                        case '7':
                            return true;
                   }
         }

   }
   return false;
}

Comments

0

If you sort the possible hex digits in a list you could build the regex automatically like so

var hexs = new List {"00", "80", "01", "81", "02"};
var regex = string.Format("^[PTQA]({0})", string.Join("|", hexs));
var accuracyCodeHexRegex = new Regex(regex);

Comments

0

If the possible values are known, why don't you compare to an array of this known possibles values?

void Foo(){

    var valueToTest1 = "P07";
    var valueToText2 = "Z54";

    TestValue(valueToTest1);
    TestValue(valueToTest1);
}

bool TestValue(string stringToTest)
{
    var hexValues = new string[] { "00", "80", "01", "81", "02", "82", "03", "83", "04", "84", "05", "85", "06", "86", "07", "87"};
    var leftValues = new char[] { 'P', 'Q', 'H' };

    var left = stringToTest[0];
    var right = strintToTest.SubString(1,2);

    return leftValues.Contains(left) && hexValues.Contains(right);
}

This is a lot simpler than using a regex and I believe far more performant

4 Comments

I have to use Regex, we have other comparisons also. Need to stick to regex.
Indeed, I doubt if regex is required and more than likely would be the least efficient solution
@Charu: what kind of other comparison may be related to this test?
if you are going to hardcode the values a switch is going to be even faster than searching in an array

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.