Anyone see the problem here?

Mardy Git

public class ErrorLogging
{
/// <summary>
/// Insert into error log
/// </summary>
/// <param name="errorMessage"></param>
/// <param name="errorTrace"></param>
/// <param name="errorLocation"></param>
public void insertErrorLog(string errorMessage, string errorTrace, string location)
{
int? errorID = 0;
XXXXX_DataAccess.common.XXXXX_CMSTableAdapters.XXXXXCMS_TA ta = new common.XXXXX_CMSTableAdapters.XXXXXCMS_TA();

        try
        {

            ta.usp\_ErrorLogInsert(ref errorID, errorMessage, errorTrace, location);

        }
        catch (Exception ex)
        {
            ta.usp\_ErrorLogInsert(ref errorID, ex.Message, ex.StackTrace, "insertErrorLog - XXXXX\_DataAccess");
        }
    }
}

BotCar

I see in my crystal ball that your future will include something called a "stack overflow". I wonder what that means?

What is this talk of release? I do not release software. My software escapes leaving a bloody trail of designers and quality assurance people in its wake.

Argonia

Hm, yes i see something wrong. It looks like a programming question in The Weird and The Wonderful. Please read carefully what the red text say.

Microsoft ... the only place where VARIANT_TRUE != true

Mardy Git

My colleague found this in some production code for a very well known company. I'm wondering what horrors we'll exhume next.

Richard Deeming

Mardy Git wrote:

<param name="errorLocation"></param>

Mardy Git wrote:

insertErrorLog(string errorMessage, string errorTrace, string location)

There it is! ;P Also, the lack of documentation for the parameters. And the camel-case method names. (Public members should always use Pascal case). And the lower-case namespace "common". And the underscores in the namespaces "XXXXX_DataAccess" and "XXXXX_CMSTableAdapters". And the underscore in the class name "XXXXXCMS_TA". But other than that, it's fine. It's not like a database insert would fail twice in a row, is it? :rolleyes:

"These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer

Sentenryu

Where? he's not calling the method recursively, the max that can happen is a unhandled SqlException.

I'm brazilian and english (well, human languages in general) aren't my best skill, so, sorry by my english. (if you want we can speak in C# or VB.Net =p) "Given the chance I'd rather work smart than work hard." - PHS241 "'Sophisticated platform' typically means 'I have no idea how it works.'"

BotCar

Oops! Misread that.

What is this talk of release? I do not release software. My software escapes leaving a bloody trail of designers and quality assurance people in its wake.

PIEBALDconsult

Needless use of a nullable?

Mardy Git wrote:

XXXXX_DataAccess.common.XXXXX_CMSTableAdapters.XXXXXCMS_TA ta = new common.XXXXX_CMSTableAdapters.XXXXXCMS_TA();

Inconsistent use of fully qualified name. Even I don't put empty lines at the beginning and end of a block.

Bernhard Hiller

The next gem is to be expected in the Untested Sql Procedure (usp_) ErrorLogInsert: in case of an exception happening there, it will check the location parameter, and if it starts with "insertErrorLog" not throw an exception but set errorID to null (that's why it's nullable). And the guy never checks the value of errorID after execution!