Dont trust the framework, to do the work(.net)



  • Dont trust the framework, to do the work

    I just got some ugly outsourced code in my mail, that i was assigned to work on. Its clear the developer didnt know how the framework, works.. the code is from a webservice, that CRUDs some data from a sql-server. Aparently he was affraid of the SqlClient, so he made sure nothing bad happend

    He used a variable, to state if a connection was open or not: 

                    objCon = SqlCon(); 
                    intConState=1;

                    intConState=2;
                    objCon.Close();               
                    intConState=3;   

    Whenever the connection was requested, he tried to open it:


            private SqlConnection SqlCon()
            {
                SqlConnection    objCon        = new SqlConnection();

                try
                {
                    objCon.ConnectionString = m_strSQLCon;               
                    objCon.Open();                   
                }
                catch(System.Exception objException)
                {               
                    throw objException;
                }
                return objCon;
            }


    When Calling a single stored procedure, he used a transaction

    If a client requested some data, he didnt trust the build-in serialization and passed objects, so he did it himself.. Loaded the result from the sqlserver to a dataset, wrote the xml from the dataset (using WriteXml) to a memorystream. Loaded the xml into a XmlDocument, and finally returned it as a string using XmlDoc.OuterXml .. On the clientside he parsed the xmldocument manually.



  • Catch and then throw? Like baseball???



  • @Ice^^Heat said:

    Catch and then throw? Like baseball???

       Well, kinda.  Except that it's the /right/ thing to do, with a baseball.

      



  • Why do they always throw away the call stack?



  • @bobday said:

    Why do they always throw away the call stack?

    That stuff is like a hot potato! You don't want to keep it in your hands, now do you?



  • I wish the worst transgression at my place-of-work was throwing away the call stack.  One developer (who, incidentally, is now my manager) used a nice idiom:

    try {
       ...
    } catch ( Exception ex ) {
       throw new Exception( "Something failed", ex.InnerException );
    }

     

    I suppose it's one step up from ignoring the exception entirely.
     



  • We get no love from exceptions that happen deep in our own custom framework. Most of it is compiled and distributed as assemblies for the regular developers to use. Pretty much everything that breaks results in an "Object reference not set to an instance of an object" exception.



  • Uggh.  I especially like the Hungarian Notation.  That's a nice touch.




  • @samael said:

    I wish the worst transgression at my place-of-work was throwing away the call stack.  One developer (who, incidentally, is now my manager) used a nice idiom:

    try {
       ...
    } catch ( Exception ex ) {
       throw new Exception( "Something failed", ex.InnerException );
    }

     

    I suppose it's one step up from ignoring the exception entirely.
     

     

    More like a step sideways...



  • @lpope187 said:

    Uggh.  I especially like the Hungarian Notation.  That's a nice touch.


    I call that particular flavor "microhungarian", since it was the vbscript team who bequeathed the infinitely reusable "obj" prefix on the world.



  • > he used a transaction

    To be fair, in some circumstances this can be legit, especicially if the SP was designed by a muppet, or you want to control the isolation level.

    But given the rest of the code I don't feel the need for much benefit of the doubt...



  • Code monkey catch exception. Code monkey know exception bad! Code monkey throw bad exception away. No exception, manager Rob happy! Manager give code monkey extra.



  • Could it be that you don't understand the purpose of rethrowing exceptions? Seems so, since you're showing bad examples.
    Here's a good one:

    public class XNodeList extends AbstractList<Node> implements List<Node> {

    private final NodeList l;

    public XNodeList(NodeList l) {
    this.l = l;
    }

    public boolean add(Node n) {
    try {
    this.get(this.size()).getParent().appendChild(n);
    } catch (DOMException e) {
    if(e.code == NO_MODIFICATION_ALLOWED_ERRR || e.code == NOT_SUPPORTED_ERR) {
    throw new UnsupportOperationException("adding Node "+n.id+" to NodeList is not allowed",e);
    } else {
    throw new IllegalArgumentException("failed to add Node "+n.id+", e);
    }
    }
    return true;
    }
    ...

    }



  • He catches the exception, but he doesnt handle it anyway. Then why do it? And by saying Throw Exception, he removes the Call-stack. He should simply have written, throw, even better, he shouldn't have catched it in the first place

    He also used exceptions for logik, which is a no-go

    See http://msdn2.microsoft.com/en-us/library/ms229005.aspx



  • @samael said:

    I wish the worst transgression at my place-of-work was throwing away the call stack.  One developer (who, incidentally, is now my manager) used a nice idiom:

    try {
       ...
    } catch ( Exception ex ) {
       throw new Exception( "Something failed", ex.InnerException );
    }

     

    I suppose it's one step up from ignoring the exception entirely.
     

    That's close to how you implement a Paula.NoQuackException.



  • @duckie said:

    When Calling a single stored procedure, he used a transaction

     

    I think this isn't necessarily bad (when the SP doesn't contain transaction code and you can't change the SP), after all if the SP fails midway through, it could leave your db in an undefined state. Or am I wrong?
     



  • You are right, but wouldnt that be a WTF in the SP? ;-)



  • @duckie said:

    He catches the exception, but he doesnt handle it anyway. Then why do it? And by saying Throw Exception, he removes the Call-stack. He should simply have written, throw, even better, he shouldn't have catched it in the first place

    He also used exceptions for logik, which is a no-go

    See http://msdn2.microsoft.com/en-us/library/ms229005.aspx

     I like it... :)


Log in to reply