C# if statement with string.Contains() not working as expected

Question

I am stuck on what I thought would be a really easy problem. I am trying to redirect a user to a different website if the UserAgent does not contain a series of strings. The part I can not figure out is that the if statement works fine if I use the code below. I am happy with the results with this but something tells me that it is not good practice to only have an else statement and perform nothing if the statement proves to be true.

        string strUserAgent = Request.UserAgent.ToString().ToLower();

        if (strUserAgent != null)
        {
            if (Request.Browser.IsMobileDevice == true || 
                strUserAgent.Contains("iphone") ||
                strUserAgent.Contains("blackberry") || 
                strUserAgent.Contains("mobile") ||
                strUserAgent.Contains("windows ce") || 
                strUserAgent.Contains("opera mini") ||
                strUserAgent.Contains("palm") ||
                strUserAgent.Contains("android"))
            {
                // Is this normal practice to only have an else block?
            }else
            {
                Response.Redirect(AppState["redirectBack"].ToString());
            }

When I try the next block of code the script redirects the user no matter what the UserAgent string contains. Can someone explain why this might be happening?

        string strUserAgent = Request.UserAgent.ToString().ToLower();

        if (strUserAgent != null)
        {
            if (Request.Browser.IsMobileDevice != true || 
                !strUserAgent.Contains("iphone") ||
                !strUserAgent.Contains("blackberry") || 
                !strUserAgent.Contains("mobile") ||
                !strUserAgent.Contains("windows ce") || 
                !strUserAgent.Contains("opera mini") ||
                !strUserAgent.Contains("palm") ||
                !strUserAgent.Contains("android"))
            {
                 Response.Redirect(AppState["redirectBack"].ToString());
            }

By the way, instead of having this mammoth contains block, why don't you put all of the strings into a collection, and just do a Contains on that? It would drastically simplify it. — Arran
– Arran, Commented Nov 1, 2013 at 16:20
You've inverted it incorrectly. By changing == to != and Contains to !Contains but leaving the ||s as-is, you're only entering the block if all of the Contains are true. You can change all of the ||s to &&s, or undo the individual negations and wrap the whole thing with !(). — Cloudy
– Cloudy, Commented Nov 1, 2013 at 16:26

Jeroen Vannevel · Accepted Answer · 2013-11-01 16:24:26Z

8

Invert your statement using ! ("not"):

if(!(conditions)) { }

This will avoid the need to use an empty code block and you can just drop the else.

Your second codeblock will redirect when you're not on a mobile device or when your useragent contains any of the following strings. It depends on your input and your environment.

Do note that it's a lot easier to create a collection of the possibilities and check if your useragent is in there:

if(new[] {"iphone", "somephone", "otherphone" }.Any(x => useragent.Contains(x))) {}

edited Nov 1, 2013 at 16:24

answered Nov 1, 2013 at 16:18

Jeroen Vannevel

44.6k25 gold badges114 silver badges174 bronze badges

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

4 Comments

Tim S. Over a year ago

Depending on which is easier to read, you could wrap the whole thing in a ! or reverse each boolean (!=, !strUserAgent.Contains(...)) and swap || for &&.

Joachim Isaksson Over a year ago

The contains you're showing requires an exact match, while the question matches if the user agent contains one of the stings.

Tim S. Over a year ago

That Array.Contains is backwards. You could do new[] { ... }.Any(x => useragent.Contains(x)).

Jeroen Vannevel Over a year ago

Overlooked that part, good point. I've edited it, thanks for the elegant solution.

cadrell0 · Accepted Answer · 2013-11-01 16:23:29Z

8

You need De Morgan's Law. When you inverted your condition, your ORs need to become ANDs

answered Nov 1, 2013 at 16:23

cadrell0

17.4k5 gold badges55 silver badges70 bronze badges

3 Comments

Satpal Over a year ago

+1, I was thinking exactly same, This is best answer of the lot. It is much easier to giver programmatic answer

cadrell0 Over a year ago

@Satpal AllenG is correct as well, and he is not too lazy to type out the code.

AllenG Over a year ago

@cadrell0 - normally I am. I'm just in a giving mood this morning.

AllenG · Accepted Answer · 2013-11-01 16:23:17Z

It will always be true that at least one of your conditions will not be true. For instance, if strUserAgent.Contains(iphone) will be false if strUserAgent.Contains("blackberry") is true.

You need to change your OR (||) operator to a logical AND (&&) operator.

    if (strUserAgent != null)
    {
        if (Request.Browser.IsMobileDevice != true && 
            !strUserAgent.Contains("iphone") &&
            !strUserAgent.Contains("blackberry") && 
            !strUserAgent.Contains("mobile") &&
            !strUserAgent.Contains("windows ce") && 
            !strUserAgent.Contains("opera mini") &&
            !strUserAgent.Contains("palm") &&
            !strUserAgent.Contains("android"))
        {
             Response.Redirect(AppState["redirectBack"].ToString());
        }

nawfal · Accepted Answer · 2013-11-01 16:32:33Z

3

Not an answer, just a suggestion. You can make your code cleaner with an extension method:

public static bool ContainsAnyOf(this string source, params string[] strings)
{
    return strings.Any(x => source.Contains(x));
}

And now write

if (strUserAgent.ContainsAnyOf("iphone", "blackberry", "mobile", "windows ce", "opera mini", "palm", "android"))
{
     //
}

answered Nov 1, 2013 at 16:32

nawfal

73.8k59 gold badges342 silver badges379 bronze badges

Comments

Harrison · Accepted Answer · 2013-11-01 16:22:27Z

0

You need to reverse the entire thing. Putting !A || !B is not the same as !(A||B). In the first one if its A then it's Not B so it's True. In the second one it's False.

   if (!(Request.Browser.IsMobileDevice == true || 
            strUserAgent.Contains("iphone") ||
            strUserAgent.Contains("blackberry") || 
            strUserAgent.Contains("mobile") ||
            strUserAgent.Contains("windows ce") || 
            strUserAgent.Contains("opera mini") ||
            strUserAgent.Contains("palm") ||
            strUserAgent.Contains("android")
      ) )
        {
            Response.Redirect(AppState["redirectBack"].ToString());
        }

answered Nov 1, 2013 at 16:22

Harrison

3,9837 gold badges24 silver badges50 bronze badges

Comments

Sudhakar Tillapudi · Accepted Answer · 2013-11-01 16:23:11Z

0

 string strUserAgent = Request.UserAgent.ToString().ToLower();

    if (strUserAgent != null)
    {
        if (!(Request.Browser.IsMobileDevice == true || 
            strUserAgent.Contains("iphone") ||
            strUserAgent.Contains("blackberry") || 
            strUserAgent.Contains("mobile") ||
            strUserAgent.Contains("windows ce") || 
            strUserAgent.Contains("opera mini") ||
            strUserAgent.Contains("palm") ||
            strUserAgent.Contains("android")))            
        {
            Response.Redirect(AppState["redirectBack"].ToString());
        }

answered Nov 1, 2013 at 16:23

Sudhakar Tillapudi

26.3k5 gold badges43 silver badges70 bronze badges

Collectives™ on Stack Overflow

C# if statement with string.Contains() not working as expected

6 Answers 6

4 Comments

3 Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

4 Comments

3 Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related