Is it possible to simplify this try-catch and if logic?

Question

I have the following code. It's working as intended, but I just wanted to know if I can change the logic behind it to make it look simpler because I believe it's redundant.

It works like this: I fetch some data and then try to pass it through a regular expression. There are three possible outcomes:

Data is fetched ok.
Data is fetched but it doesn't contain a number. So another regex is passed.
Data is not fetched. Same regex as 2) is passed. (this is what I believe could be simplified or optimised)

At first I thought that there could be a way of checking if the regular expression returns empty or not without throwing an exception, but (correct me if I'm wrong) there isn't such a thing... so that's why I included the try/catch. Without it, if the 3rd case is met, the IF returns an exception because it says that the m2[0].Groups[1].Captures[0].Value is out of bounds, of course (because it's empty).

So... is there any way to make this look more optimised? Thank you very much!

string regexString = @"&nbsp;\.\.\.&nbsp;.*?>(.*?)<\/a>";
MatchCollection m2 = Regex.Matches(myInput, regexString, RegexOptions.Singleline);

try
{
    if (m2[0].Groups[1].Captures[0].Value.All(Char.IsDigit) == false)
    {
        regexString = @"page=.*?"">(.*?)</a>\n.*?<a class=""pagebtn""";
        m2 = Regex.Matches(myInput, regexString);
    }
}
catch (ArgumentException)
{
    regexString = @"page=.*?"">(.*?)</a>\n.*?<a class=""pagebtn""";
    m2 = Regex.Matches(myInput, regexString);
}

You should solve the exception. Which particular statement throws, `m2[0].Groups[1].Captures[0] .Value` I guess? That's a string, so a `string.IsNullOrWhiteSpace()` would suffice. A `Regex` instance has a `bool IsMatch` property you can check. — CodeCaster, Sep 06 '15 at 10:12
The only way of solving it would be checking if the regex result is empty. And that is part of my question because I have no idea if that can be done. This is why I am using try/catch. — Arturo, Sep 06 '15 at 10:13
@CodeCaster Using Regex.IsMatch would require me to put the exact string I am expecting to get, wouldn't it? But the thing is that I only know that it has to be an integer, and nothing else. — Arturo, Sep 06 '15 at 10:15
I was mistaken, it is [`Match.Success`](https://msdn.microsoft.com/en-us/library/system.text.regularexpressions.match(v=vs.110).aspx) you should check, so `m2[0].Success`. — CodeCaster, Sep 06 '15 at 10:16
http://stackoverflow.com/questions/1732348/regex-match-open-tags-except-xhtml-self-contained-tags — AgentFire, Sep 06 '15 at 10:17
@AgentFire I know, I know, but it's just to get a number from a webpage. I know I'm going to hell for this, don't worry ;) — Arturo, Sep 06 '15 at 10:17
It is better to avoid using exceptions for control-flow where possible. When I'm auditing a code base, I'm looking for places where the previous programmer has swallowed exceptions, and practices like this make the job harder. You can use extension methods to turn this into `var allDigits = m2.Safely(0, m=> m.Groups.Safely(1, g => g.Captures.Safely(0, c => c.Values.All(Char.IsDigit))))` — Chui Tey, Sep 06 '15 at 10:19

Icepickle · Answer 1 · 2015-09-06T10:26:13.070

2

You could check it with IsMatch and if it doesn't match, try your second regex, if not, nothing matches

Regex normalRegex = new Regex(@"&nbsp;\.\.\.&nbsp;.*?>(.*?)<\/a>", RegexOptions.SingleLine);
Regex secondaryRegex = new Regex(@"page=.*?"">(.*?)</a>\n.*?<a class=""pagebtn""");
int valueFound;
bool numberParsed;

if (normalRegex.IsMatch(myInput)) {
   // your code to check here
   // use int.TryParse to parse your value and add the result to 
   numberParsed = int.TryParse(m2[0].Groups[1].Captures[0].Value, out valueFound);
}
if (!numberParsed) {
    if (secondaryRegex.IsMatch(myInput)) {
        // second code matches
    } else {
        // no match
    }
}

In that case you don't really need your try / catch

edited Sep 06 '15 at 10:26

answered Sep 06 '15 at 10:18

Icepickle

12,689
3
34
48

With IsMatch I would need to know exactly what is the result. Shouldn't I use .Success as the other answer suggests? – Arturo Sep 06 '15 at 10:22
1

maybe so, but you could easily adapt the logic of it, I editted my post @Arturo – Icepickle Sep 06 '15 at 10:26

score 2 · Accepted Answer · answered Sep 06 '15 at 10:19

I'm assuming that a more accurate description of your problem is that you wish to parse a value that may or may not be there as an integer, but that your m2[0].Groups[1].Captures[0].Value.All(Char.IsDigit) throws if Value is null.

That can then be simplified to something like this:

int parsedValue = 0;
if (m2[0].Success)
{
    var value = m2[0].Groups[1].Captures[0].Value;
    if (!string.IsNullOrWhiteSpace(value))
    {
        if (int.TryParse(value, out parsedValue))
        {
            // do something with parsedValue, or with the fact that the value exists and is an integer
        }
    }
}

Is it possible to simplify this try-catch and if logic?

2 Answers2