Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • World
  • Users
  • Groups
Skins
  • Light
  • Cerulean
  • Cosmo
  • Flatly
  • Journal
  • Litera
  • Lumen
  • Lux
  • Materia
  • Minty
  • Morph
  • Pulse
  • Sandstone
  • Simplex
  • Sketchy
  • Spacelab
  • United
  • Yeti
  • Zephyr
  • Dark
  • Cyborg
  • Darkly
  • Quartz
  • Slate
  • Solar
  • Superhero
  • Vapor

  • Default (No Skin)
  • No Skin
Collapse
Code Project
  1. Home
  2. Other Discussions
  3. The Weird and The Wonderful
  4. String Comparison

String Comparison

Scheduled Pinned Locked Moved The Weird and The Wonderful
code-review
10 Posts 9 Posters 3 Views 1 Watching
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • R Offline
    R Offline
    Rotted Frog
    wrote on last edited by
    #1

    Another one found during code review. Constants changed to protect the innocent.

    If (name.ToUpper() == "CamelCasedName".ToUpper()) {...}

    M N R Z V 5 Replies Last reply
    0
    • R Rotted Frog

      Another one found during code review. Constants changed to protect the innocent.

      If (name.ToUpper() == "CamelCasedName".ToUpper()) {...}

      M Offline
      M Offline
      Mohibur Rashid
      wrote on last edited by
      #2

      In the world of copy paste its easier to copy and pasting twice "CamelCasedName" and .ToUpper() than changing 3 character. mouse will talk ;)

      1 Reply Last reply
      0
      • R Rotted Frog

        Another one found during code review. Constants changed to protect the innocent.

        If (name.ToUpper() == "CamelCasedName".ToUpper()) {...}

        N Online
        N Online
        Nagy Vilmos
        wrote on last edited by
        #3

        Not only is at a total waste of processing, but a stupid thing to do. However bad the excessive ToUpper calls, the greater horror is using object comparison when equality is the (probable) intent. IIRC the String class overrides == but this is such bad practice the coder should be shot. Repeatedly. As an aside, comparing an object to a constant, you should always put the constant first as it removes the need for null checks:

        if ("CAMELCASENAME".equals(name.ToUpper())) {
        ...
        }


        Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett

        R V A 3 Replies Last reply
        0
        • N Nagy Vilmos

          Not only is at a total waste of processing, but a stupid thing to do. However bad the excessive ToUpper calls, the greater horror is using object comparison when equality is the (probable) intent. IIRC the String class overrides == but this is such bad practice the coder should be shot. Repeatedly. As an aside, comparing an object to a constant, you should always put the constant first as it removes the need for null checks:

          if ("CAMELCASENAME".equals(name.ToUpper())) {
          ...
          }


          Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett

          R Offline
          R Offline
          Rotted Frog
          wrote on last edited by
          #4

          Actually I completely disagree. == is not a major sin in this instance - it is fairly well understood that == is equivalent to an ordinal string comparison not an object equivalence. It is functionally equivalent to .Equals() and much more readable - I would pull up use of .Equals in a code review on the readability point. However the best option is to use String.Compare instead as this does proper culture sensitive comparisons.

          1 Reply Last reply
          0
          • N Nagy Vilmos

            Not only is at a total waste of processing, but a stupid thing to do. However bad the excessive ToUpper calls, the greater horror is using object comparison when equality is the (probable) intent. IIRC the String class overrides == but this is such bad practice the coder should be shot. Repeatedly. As an aside, comparing an object to a constant, you should always put the constant first as it removes the need for null checks:

            if ("CAMELCASENAME".equals(name.ToUpper())) {
            ...
            }


            Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett

            V Offline
            V Offline
            VallarasuS
            wrote on last edited by
            #5

            Nagy Vilmos wrote:

            you should always put the constant first as it removes the need for null checks

            you have a point, and I always prefer '.Equals' especially with strings. Often this confuses the lexical parser of the compiler itself ;P Probleminha com 'if' C# - Operator '!='[^]

            Regards Vallarasu S | FSharpMe.blogspot.com

            1 Reply Last reply
            0
            • R Rotted Frog

              Another one found during code review. Constants changed to protect the innocent.

              If (name.ToUpper() == "CamelCasedName".ToUpper()) {...}

              R Offline
              R Offline
              Rahul Rajat Singh
              wrote on last edited by
              #6

              This is how I would have done it:

              if (string.Equals(name, "CamelCasedName", StringComparison.OrdinalIgnoreCase) == true)
              {

              }

              This looks more meaningful to me and i think it will be little better too(performance wise).

              Rotted Frog wrote:

              Constants changed to protect the innocent.

              innocent :laugh:

              Every now and then say, "What the Elephant." "What the Elephant" gives you freedom. Freedom brings opportunity. Opportunity makes your future.

              C 1 Reply Last reply
              0
              • R Rotted Frog

                Another one found during code review. Constants changed to protect the innocent.

                If (name.ToUpper() == "CamelCasedName".ToUpper()) {...}

                Z Offline
                Z Offline
                ZurdoDev
                wrote on last edited by
                #7

                It's a Copy and Paste victim.

                There are only 10 types of people in the world, those who understand binary and those who don't.

                1 Reply Last reply
                0
                • N Nagy Vilmos

                  Not only is at a total waste of processing, but a stupid thing to do. However bad the excessive ToUpper calls, the greater horror is using object comparison when equality is the (probable) intent. IIRC the String class overrides == but this is such bad practice the coder should be shot. Repeatedly. As an aside, comparing an object to a constant, you should always put the constant first as it removes the need for null checks:

                  if ("CAMELCASENAME".equals(name.ToUpper())) {
                  ...
                  }


                  Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett

                  A Offline
                  A Offline
                  AspDotNetDev
                  wrote on last edited by
                  #8

                  I recommend this approach. The only time I use ToUpper or ToLower is with a switch. It's annoying that you can't use a caseless string comparer with replaces, but you can use a regular expression that is case insensitive for those.

                  Thou mewling ill-breeding pignut!

                  1 Reply Last reply
                  0
                  • R Rahul Rajat Singh

                    This is how I would have done it:

                    if (string.Equals(name, "CamelCasedName", StringComparison.OrdinalIgnoreCase) == true)
                    {

                    }

                    This looks more meaningful to me and i think it will be little better too(performance wise).

                    Rotted Frog wrote:

                    Constants changed to protect the innocent.

                    innocent :laugh:

                    Every now and then say, "What the Elephant." "What the Elephant" gives you freedom. Freedom brings opportunity. Opportunity makes your future.

                    C Offline
                    C Offline
                    CoperNick
                    wrote on last edited by
                    #9

                    Are you sure to use

                    if(Equals() == true)

                    why not

                    if((Equals()==true) == true)

                    ? Isn't this better:

                    if(Equals())

                    ?

                    1 Reply Last reply
                    0
                    • R Rotted Frog

                      Another one found during code review. Constants changed to protect the innocent.

                      If (name.ToUpper() == "CamelCasedName".ToUpper()) {...}

                      V Offline
                      V Offline
                      Vladimir Svyatski
                      wrote on last edited by
                      #10

                      I always wonder: why after many, many, many years people just can't learn to use caseless string comparison methods? I constantly see shit like this. Caseless comparison methods existed even in good old plain C. Damn, is it really that hard to master primitive things like this:

                      if (String.Compare(name, "CamelCasedName", true) == 0) {...}

                      ? In the end, it's not Lebesgue integration, right? Those, who ever had the "pleasure" to be familiar with functional analysis, know :) .

                      1 Reply Last reply
                      0
                      Reply
                      • Reply as topic
                      Log in to reply
                      • Oldest to Newest
                      • Newest to Oldest
                      • Most Votes


                      • Login

                      • Don't have an account? Register

                      • Login or register to search.
                      • First post
                        Last post
                      0
                      • Categories
                      • Recent
                      • Tags
                      • Popular
                      • World
                      • Users
                      • Groups