Mircosoft count don't trust



  • Yesterday I was asked to make a minor change to a page on our online app.  Whilst trolling the code for this particular page, I came across this little gem written by a contractor who left us about a month ago:

     // Counts the xml node
    private int CountXmls(XmlDocument xml)
    {
        try
        {
            int nodecount = 0;
            foreach (XmlNode node in xml.DocumentElement.ChildNodes)
            {
                // mircosoft count don't trust
                nodecount = nodecount + 1;
                
                if (node.ChildNodes.count > 0)
                {
                    nodecount = nodecount + node.ChildNodes.count;
                }            
            }
        }
        catch (Exception ex)
        {
            try
            {
                ErrorLogger.Log("The XMl count didn't worked");
            }
            catch
            {
                // What can We do if Logger failed
            }
        }
       
        return nodecount;
    }

    The contractor in question was always talking about how bad .NET sucks and how he would have designed it better.  Ironically, he left us for a contract gig at Microsoft.

    I know this is a minor collection of WTFs compared to some of the abominations I've seen on TDWTF, but it bothered me enough that I had to post it.  For those of you who are .NET devs, I know you feel my pain.  For those of you who aren't, most (all?) of the .NET collection classes have a .count or .length property to do exactly what this craptastic routine tried, and epic failed, to achieve.



  • TRWTF is letting that compiler warning go by. "Variable ex is never used." Aren't all dev's anal retentive about that like I am?

    Moderator's note: Removed the full quote of the OP.



  • @Nexzus said:

    TRWTF is letting that compiler warning go by. "Variable ex is never used." Aren't all dev's anal retentive about that like I am?
     

    Given the number of WTFs committed here, a missed compiler warning doesn't shock me.  Or is my sarcasm detector failing me this morning? :D



  • @Smitty said:

    @Nexzus said:

    TRWTF is letting that compiler warning go by. "Variable ex is never used." Aren't all dev's anal retentive about that like I am?
     

    Given the number of WTFs committed here, a missed compiler warning doesn't shock me.  Or is my sarcasm detector failing me this morning? :D

    Possibly, I can't remember if it was intended to be sarcasm. It's early here as well.



  • @Nexzus said:

    Ironically, he left us for a contract gig at Microsoft.
    Uh-oh. Time to buy Apple stock.


  • BINNED

    @Smitty said:


    {
        try
        {
            int nodecount = 0;
            [...]
        } [...]
       
        return nodecount;
    }
     

    I don't know C# but isn't that variable out of scope at the return statement??
    And "CountXmls" is a great name, too.



  • @topspin said:

    I don't know C# but isn't that variable out of scope at the return statement??
     

     Yeah that was my fault.  I was reading the code from one machine and typing it into this one.  Pardon my dyslexia. :D

     I too enjoyed the "CountXmls" method name, the useless nested Try, the epic failure of an error message, etc.



  • @Smitty said:

    @topspin said:

    I don't know C# but isn't that variable out of scope at the return statement??
     

     Yeah that was my fault.  I was reading the code from one machine and typing it into this one.  Pardon my dyslexia. :D

     I too enjoyed the "CountXmls" method name, the useless nested Try, the epic failure of an error message, etc.

    Indeed, we have a nested WTF:   "What can We do if Logger failed"

    catch (Exception ex)
        {
            try
            {
                ErrorLogger.Log("The XMl count didn't worked");
            }
            catch
            {
                // What can We do if Logger failed
            }
        }



  • @Smitty said:

    For those of you who are .NET devs, I know you feel my pain.  For those of you who aren't, most (all?) of the .NET collection classes have a .count or .length property to do exactly what this craptastic routine tried, and epic failed, to achieve.

    I think I know what he tried to achieve. If you look carefully, you can see that he does infact use ChildNodes.Count, the purpose of this was to find out how many total nodes there are, including sub-nodes. This only works if you have two levels of nodes.

    Something better would be perhaps:

    private int TotalChildNodes(XmlNode node)
    {
        int nodecount = node.Childnodes.Count;
    
        foreach (XmlNode child in xml.ChildNodes)
        {
            nodecount += TotalChildNodes(child);
        }
        
        return nodecount;
    }
    


  • I have had cases in the past where a "Count" method did not actually return the correct count, because I think it was based on some cached value rather than the actual value. I think this was using some kind of Microsoft component to access DBF files.



  • Must resist urge to implement recursive voodoo...



  • @SlyEcho said:

    If you look carefully, you can see that he does infact use ChildNodes.Count,
     

     I found it amusing that he had written an engrish comment about not trusting Microsoft's count method and then immediately called said method.



  • @Smitty said:

     // mircosoft count don't trust
                nodecount = nodecount + 1;
                
                if (node.ChildNodes.count > 0)
                {
                    nodecount = nodecount + node.ChildNodes.count;
                }      

    Don't trust .Count! Not until the next line, anyway.


Log in to reply