• Steve (unregistered)

    Also, unnecessarily passing byval

  • (nodebb)

    I may be repeating the post above me which is currently held for moderation.

    Makes me wonder if that codebase started in VB where On Error Resume Next was a common (if bad) idiom, and these method-level try with do-nothing catches became the way to retain that (bad) idiom in code during the early years of the conversion.

    And now they're stuck with it.

    Or, far more likely, this is simply the work of over-worked under-resourced abject incompetents. Just like everything else featured on this site.

  • (nodebb)

    Becuase it's VB, it would return False rather than null.

  • (nodebb) in reply to WTFGuy

    In my experience, empty catches come from people who don't know what they are doing, but try to take the advice "always catch exceptions". It's bad advice, and the implementation makes the code worse than if they did nothing, but it's still what I hear from the people that I see do it.

  • (nodebb) 
    Private Function errorCheck() {
try {
    programmingLanguage.explain();
      }
catch {
    web.submit("https://thedailywtf.com",this);
      {
return FileNotFound;
}

    Addendum 2024-03-26 09:34: EDIT: Dang you, markdown!

  • (nodebb)

    is that every function was implemented this way. Every function had a Try/Catch that frequently did nothing, or rarely (usually when they copy/pasted from StackOverflow)

    In my Stack Overflow answers I always put something in may catch blocks

    do
{
    try somethingThatMightThowButAnswersTheQuestion()
}
catch 
{
    // Handle your errors here
}

    If you see Swift code with a catch block with a comment in it like that, it was probably copy-pasta'd from one of my SO answers.

  • (nodebb)

    If every function uses this try/catch pattern, then if the function returns an unexpected null the caller will ignore it, and errors will be ignored all the way up. If the goal is to prevent the application from crashing with an "uncaught error" exception, it "works".

  • Sauron (unregistered)

    Swallowing all exceptions? But that's "standard industry practice" :D

    The code probably relies on that to pass the tests (if any), so the code seems to work, and only fails in prod, so the manglement cannot easily pinpoint who to blame when horrible things happen.

  • (nodebb) 
    queryEmail = FnsEmail

    Done.

    But seriously, what's up with all those naming issues - VS calls them out since 2005 IIRC.

    Addendum 2024-03-26 13:45: BTW don't use String.Equals(queryEmail, FnsEmail); in .net you always use the optimized operators before the 40 times slower virtual Equals call that also wastes additional performance with value types because it has to box/unbox them.

  • Oracles (unregistered)

    A classic case of paid-by-the-pound code.

  • (nodebb)

    Of course, since emails that differ only in casing are functionally equivalent, the comparison probably ought to be case insensitive. Probably. Depends on how thoroughly the requirements were written (Hah!) and exactly why we're comparing these two fields.

    Culture gets a look in, since without knowing where these strings come from we can't guarantee there aren't culture issues too.

    So Return String.Equals(queryEmail, FnsEmail, StringComparison.InvariantCultureIgnoreCase) is more correct. If we are certain that e.g. QueryEmail is always non-null that can be more concisely restated as Return queryEmail.Equals( FnsEmail, StringComparison.InvariantCultureIgnoreCase)

    I'll take some exception to MaxiTB's suggestion to always use the == and != or VB.Net <> (hope my guess at formatting works; that's lessthan followed by greaterthan characters) operators.

    See https://learn.microsoft.com/en-us/dotnet/api/system.string.op_equality?view=net-8.0 which states in pertinent part:

    Remarks The Equality method defines the operation of the equality operator for the String class. It enables code such as that shown in the Example section. The operator, in turn, calls the static Equals(String, String) method, which performs an ordinal (case-sensitive and culture-insensitive) comparison.

    Note

    The Visual Basic compiler does not resolve the equality operator as a call to the Equality method. Instead, the equality operator wraps a call to the Operators.CompareString method.

    So it seems there's not as much deeply optimized mumbo jumbo behind the operators as one might hope. There might be an advantage in the way the compiler susses out the argument types and thereby avoids calling overloads that trigger unnecessary type conversion / coercion. But if so, that's not documented behavior and therefore non-contractual.

    And of course if case- or culture-insensitive comparison is required, then == or != is simply incorrect. Faster but wronger is not an improvement.

  • (nodebb) in reply to WTFGuy

    emails that differ only in casing are functionally equivalent

    One of the often made assumptions about email addresses that are wrong.

  • Gavin (unregistered)

    One could make a case for creating a function such as Mailid_compare() to encapsulated Mailid comparisons. It becomes immediately apparent what we are comparing and allows for more complex comparisons - maybe mail aliases are to be considered equivalent, etc.. Even if this is not the case in this particular snippet.

    But yes, TRWTF is needless exception handling.

  • TheCPUWizard (unregistered)

    Back in the dark ages when I was in the role, I used to require teams to provide a "document" of why they needed the try catch...

    1. Why was the condition "exceptional" (i.e. they could not pre-check.
    2. What is the corrective measure that is effectively take at this level Having to answer these two questions inevitably let to better designs [note: 'better' means incremental improvement, does no imply ascension to 'good']
  • (nodebb) in reply to WTFGuy

    since emails that differ only in casing are functionally equivalent

    Only mostly true for domain names (only definitely true for the ASCII ones) and it's up to the receiving system to decide how to map from incoming mailbox name to where to store the message; that could be done in a case-insensitive way, but there really is no guarantee of that at all.

  • Officer Johnny Holzkopf (unregistered) in reply to Oracles

    Combined with a severe case of copypasta, probably from some documentation (commonly stored in a Word document or Excel table) that says something like "to compare strings, use String.Compare(str1, str2)", so the names str1 and str2 needed to be kept because the documentation said so...

  • Craig (unregistered)

    It perhaps should be noted that "On Error Resume Next" is equivalent to having a do-nothing Try/Catch on every single statement individually. The single catch-all is more akin to an "On Error GoTo" where the target just falls off the end of the function.

    There were some specific instances in VBA where it makes sense to use, when it's immediately followed by a check on the current error code with appropriate processing.

Leave a comment on “Exceptional String Comparisons”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article