Check for all zeros


  • :belt_onion:

    When doing a code review for a library developed by an offshore company I came across the following parsing function.<o:p></o:p>

    It's purpose is to take a string containing a datetime from mainframe and convert it to a valid .NET datetime object. If mainframe doesn't have a date it can either leave the field empty or fill it with zeros.<o:p></o:p>

     <o:p></o:p>

    public static DateTime? GetDateTime(this string value, DateTimeFormat format)<o:p></o:p>

    {<o:p></o:p>

        if (string.IsNullOrEmpty(value) || string.IsNullOrEmpty(value.Trim()))<o:p></o:p>

        {<o:p></o:p>

            return new DateTime?();<o:p></o:p>

        }<o:p></o:p>

     <o:p></o:p>

        //check for all zeros<o:p></o:p>

        string zero = value.Trim();<o:p></o:p>

        int length = zero.Length;<o:p></o:p>

        string check = new StringBuilder().Append('0', length).ToString();<o:p></o:p>

        if (zero == check)<o:p></o:p>

        {<o:p></o:p>

            return new DateTime?();<o:p></o:p>

        }<o:p></o:p>

     <o:p></o:p>

        //snip<o:p></o:p>

        //...<o:p></o:p>

     <o:p></o:p>

    }<o:p></o:p>



  •  return new DateTime_wtf_!




  • God leisure suit larry, what game was the death of him, I can't remember, I guess when better graphics were invented.

     

    Sorry...

     

    You get what you pay for :P



  • ToString on a known string, not just runing trim once instead of twice, probably screwing up how you are suposed to use stringbuilder, where is the wtf?

    I only see minor issues.



  • @henke37 said:

    ToString on a known string, not just runing trim once instead of twice, probably screwing up how you are suposed to use stringbuilder, where is the wtf? I only see minor issues.

     

    In case you are being sarcastic, doing this

    <FONT size=2><FONT face="Courier New">string check = new StringBuilder().Append('0', length).ToString();</FONT></FONT>

    <FONT size=2><FONT face="Courier New">if (zero == check)</FONT></FONT>

    <FONT face="Courier New" size=2>

    </FONT>

    <FONT face="Courier New" size=2>instead of </FONT>

    <FONT face="Courier New" size=2>if(value.Trim('0')=="")</FONT>


  • :belt_onion:

    @campkev said:

    In case you are being sarcastic, doing this

    <font size="2"><font face="Courier New">string check = new StringBuilder().Append('0', length).ToString();</font></font>

    <font size="2"><font face="Courier New">if (zero == check)</font></font>

    <font face="Courier New" size="2">

    </font>

    <font face="Courier New" size="2">instead of </font>

    <font face="Courier New" size="2">if(value.Trim('0')=="")</font>

    Nice solution. I hadn't thought of that one. I'm coming from a C++ background where I would have created a loop over the characters. But I didn't feel right to use that approach in C#.

    Not that it matters that much. I wont ask our offshore developers to change the code. We are to deliver in about 10 days and since their function produces the expected the results, my boss wants them to focus on the functional issues instead of the technical ones.



  • I wonder if it ever returns null.



  • @bjolling said:

    I'm coming from a C++ background where I would have created a loop over the characters.
     

    Even C++ has a better way of doing this:

    if(value.find_first_not_of('0')==std::string::npos) //either empty or all '0's. 


  • if (value.All(c => c == '0'))

    same as looping but functional!



  • I prefer Perl:



    if(value !~ /^0*$/)



  • I particularly like how it bothers to create a [code]new DateTime()?[/code] instead of just returning [code]null[/code].

    Then of course there's the bigger question of why it doesn't just use [code]DateTime.TryParse[/code], with a custom [code]IFormatProvider[/code] if necessary...


  • :belt_onion:

    Well, that migrated badly...


  • Notification Spam Recipient

    @bjolling said in Check for all zeros:

    Well, that migrated badly...

    Yeah, apparently the tag must not be allowed...

    Edit: It's not:
    0_1468394227564_upload-58c64aab-659c-41ba-a3a6-305e91b51095



  • OP converted to NodeBB/Markdown:

    When doing a code review for a library developed by an offshore company I came across the following parsing function.

    It's purpose is to take a string containing a datetime from mainframe and convert it to a valid .NET datetime object. If mainframe doesn't have a date it can either leave the field empty or fill it with zeros.

    public static DateTime? GetDateTime(this string value, DateTimeFormat format)
    {
        if (string.IsNullOrEmpty(value) || string.IsNullOrEmpty(value.Trim()))
        {
            return new DateTime?();
        }
     
        //check for all zeros
        string zero = value.Trim();
        int length = zero.Length;
        string check = new StringBuilder().Append('0', length).ToString();
        if (zero == check)
        {
            return new DateTime?();
        }
     
        //snip
        //...
     
    }


  • @Helix said in Check for all zeros:

     return new DateTime_wtf_!


    Gosh I was drinking heavily in those days - I couldn't even type without spewing tags


  • :belt_onion:

    @Helix Who would have thought 7 years ago that we'd all be sittin' here typing NodeBB/Markdown
    In them days, we'd a' been glad to spew tags about



  • Nowadays we have String.IsNullOrWhiteSpace(). What a time to be alive!



  • I could write IsNullOrWhiteSpaceOrZeroes and put it as a library on npm


  • :belt_onion:

    We never used to have libraries. We used to have to write every function again for every project we did.
    But you know, we were happy in those days, though we were poor.


  • Discourse touched me in a no-no place

    @fbmac said in Check for all zeros:

    I could write IsNullOrWhiteSpaceOrZeroes and put it as a library on npm

    If you do that, please use a strange license too. 🎛



  • @dkf Thinking about licenses is hard, I'll just use that latest agpl that there is around. Newer is always better.



  • @fbmac said in Check for all zeros:

    Newer is always better.

    This mentality is what keeps the tech industry in business.


  • BINNED

    @Gurth said in Check for all zeros:

    This mentality is what keeps the tech industry in business.a pile of shit


  • Discourse touched me in a no-no place

    @Luhmann Why didn't you change any of that statement with your <del></del><ins></ins> tags?



  • @djls45 said in Check for all zeros:

    OP converted to NodeBB/Markdown:

    When doing a code review for a library developed by an offshore company I came across the following parsing function.

    It's purpose is to take a string containing a datetime from mainframe and convert it to a valid .NET datetime object. If mainframe doesn't have a date it can either leave the field empty or fill it with zeros.

    public static DateTime? GetDateTime(this string value, DateTimeFormat format)
    {
        if (string.IsNullOrEmpty(value) || string.IsNullOrEmpty(value.Trim()))
        {
            return new DateTime?();
        }
     
        //check for all zeros
        string zero = value.Trim();
        int length = zero.Length;
        string check = new StringBuilder().Append('0', length).ToString();
        if (zero == check)
        {
            return new DateTime?();
        }
     
        //snip
        //...
     
    }
    

    So the caller provided the format and the DateTime was nullable. Did the function ever return null or did it just return a nullable object? Does it always return a new DateTime?() if it can't parse the string or only for empty and zero filled strings?


  • :belt_onion:

    @GinoMan said in Check for all zeros:

    @djls45 said in Check for all zeros:

    So the caller provided the format and the DateTime was nullable. Did the function ever return null or did it just return a nullable object? Does it always return a new DateTime?() if it can't parse the string or only for empty and zero filled strings?

    IIRC their chief architect had read an article on some pattern (Null Object Pattern?) where functions would never return null. Instead they would return an uninitialized object so that the calling function could never crash with a NullReferenceException.

    I can imagine it's a good idea to return an empty IEnumerable instead of null so that writing a loop is easier... but for other cases, just let the calling function do a proper check on null

    The team in India felt that having the application blow up in the face of the customer was not good for their reputation. Writing unmaintainable code that is expensive to extend on the other hand is less visible. That's a problem for another day.


  • Discourse touched me in a no-no place

    @bjolling said in Check for all zeros:

    The team in India felt that having the application blow up in the face of the customer was not good for their reputation.

    Handle a failure badly, get a different sort of failure elsewhere where it causes more trouble.



  • @bjolling said in Check for all zeros:

    I can imagine it's a good idea to return an empty IEnumerable instead of null

    nil acts like an empty read-only version of most Go data structures. Slices and maps are fairly straightforward, but nil channels never resolve, and that can be useful in some cases.

    Also, in Go, nil pointers only give you trouble when you dereference them, not when you call a method on them. So you can make a type that gracefully handles that.


  • Discourse touched me in a no-no place

    @ben_lubar said in Check for all zeros:

    Also, in Go, nil pointers only give you trouble when you dereference them, not when you call a method on them.

    So it always uses compile-time-resolved method dispatch?



  • @dkf said in Check for all zeros:

    @ben_lubar said in Check for all zeros:

    Also, in Go, nil pointers only give you trouble when you dereference them, not when you call a method on them.

    So it always uses compile-time-resolved method dispatch?

    It sounds more like to me that it does what Objective-C does with method dispatch, you can send messages to nil objects all day long and nothing will happen. But if you try to dereference a nil pointer, then you'll segfault. Objective-C (and likely Go, I don't know for certain as I've not studied it) is runtime (late) bound as if all its methods were C++ methods declared as "virtual" (obviously there are differences but that's the general gist of it).



  • @fbmac said in Check for all zeros:

    I could write IsNullOrWhiteSpaceOrZeroes and put it as a library on npm

    Just remember to revoke it once it becomes popular...



  • @dkf There are two ways to call a method on nil: an interface or something other than an interface.

    Calling a method on the nil interface crashes with a stack trace and all that. Calling a method on a nil pointer, slice, channel, map, etc. works unless the method dereferences some pointer.

    Confusingly, you can end up with a nil pointer inside a non-nil interface:

    var x *int = nil
    var y interface{} = x
    

    Only named types and pointers to named types can have methods. None of the types defined by default (int, string, etc.) have methods defined on them.


  • :belt_onion:

    @GinoMan said in Check for all zeros:

    @dkf said in Check for all zeros:

    @ben_lubar said in Check for all zeros:

    Also, in Go, nil pointers only give you trouble when you dereference them, not when you call a method on them.

    So it always uses compile-time-resolved method dispatch?

    It sounds more like to me that it does what Objective-C does with method dispatch, you can send messages to nil objects all day long and nothing will happen.

    The "send messages and nothing will happen" is what scares me most. I want at least some kind of warning. If I call a function to create an object, I really want that object and if it can't create it I need to be able to see that:

    /* Step 1: Try to fetch an account with unknown ID.  
       Step 2:  Get a null object instead.
    */
    BankAccount account = BankAccountRepository.Get("unknown identifier"); 
    
    // Step 3: ???
    account.Close();
    
    // Step 4: Profit
    return true;
    

    Above is pretty much how the offshore team applied the pattern.
    #cargo-cult-programming


  • Discourse touched me in a no-no place

    @bjolling said in Check for all zeros:

    If I call a function to create an object, I really want that object and if it can't create it I need to be able to see that:

    The Go idiom would be to return a tuple of a success code and the object; it's up to the caller to check the success code. Go eschews exceptions by design.



  • @ben_lubar said in Check for all zeros:

    There are two ways to call a method on nil: an interface or something other than an interface.

    There are two models of car: Ford and everything other than Ford.


  • Trolleybus Mechanic

    @fbmac said in Check for all zeros:

    @dkf Thinking about licenses is hard, I'll just use that latest agpl that there is around. Newer is always better.

    If you're putting it on npm, release it under the Sad But True license, which is:

    You may use this. However, in exactly two years, seven months and six days from (date of original release), I'm removing this from npm, just to see how many shitty programs burn


  • 🚽 Regular

    @dkf said in Check for all zeros:

    Go eschews exceptions by design.

    Nom nom nom.


Log in to reply