C# method, written by the 'senior' dev



  •  protected override void ProcessSomething(string filename, long lineNo, MyDomainObject myDomainObject, ref ReturnCodeObject result)
    {
        string originalText = myDomainObject.originalText;
        int textLen = originalText.textLength;
        int matches = 0;
        for (int i = 0; i < textLen; ++i)
        {
            char ch = originalText[i];
            if (ch == ',')
            {
                ++matches;
                break;
            }
        }
        if (matches <= 0)
        {
            myDomainObject.SetImportError(MyErrorEnums.InvalidCDRFormat);
        }
    }

    This is production code designed and written by a senior dev here.  This is just the simplest example I could find of what is a great big WTF of an application.  (.Net 3.5, C#, for anyone who cares)



  • @Anonymous Cowherd said:

    ProcessSomething

    So basically, this method should be named IgnoreAlmostAllParametersAndSetAnImportErrorIfThisFileDoesntContainAnyCommas? Or am I missing something?



  •  At least he put a break; in there

    Which makes it better than most offshored code.



  • Am I the only one who's deathly afraid of something like 'ref ReturnCodeObject result'? Reminds me so much of Win32. "Stick your object into this black hole of a function. We promise we'll take care of it!"

    I use an (Java!) API into our document management system that does something similar. Makes me feel dirty.



  • @Nexzus said:

    Am I the only one who's deathly afraid of something like 'ref ReturnCodeObject result'?

    It does make me think that the developer doesn't understand what they're doing...@Nexzus said:
    Reminds me so much of Win32. "Stick your object into this black hole of a function. We promise we'll take care of it!"
    ...and so does this.  Each and every time you send an object into a function in C# or Java, the calling function can do whatever they want with it EXCEPT tell the caller to use a separate object as the object instead... but they can change every field inside (that's not readonly).


  • @configurator said:

    So basically, this method should be named IgnoreAlmostAllParametersAndSetAnImportErrorIfThisFileDoesntContainAnyCommas? Or am I missing something?
     

    What something like this:

    if (!myDomainObject.originalText.Contains(',')) { myDomainObject.SetImportError(MyErrorEnums.InvalidCDRFormat); }

     @Nexzus said:

    Am I the only one who's deathly afraid of something like 'ref ReturnCodeObject result'?
    No.  But what you're afraid of is what actually happens to 'MyDomainObject', the error state of which is used to control program flow.  (Given the codebase this comes from it could quite easily have been both objects that are altered). This dev is also very fond of 'out' parameters.

     Oh, yeah, there's a reason all but one of the parameters are ignored - it's an attempt to write polymorphic code (the signature is required).  I'm pretty sure he thinks Liskov is a small city in the Arctic Circle.

     



  • @Anonymous Cowherd said:

    This dev is also very fond of 'out' parameters.
    Reeks of someone who doesn't understand the language. "Senior".... laugh



  • @Sutherlands said:

    ]...and so does this.  Each and every time you send an object into a function in C# or Java, the calling function can do whatever they want with it EXCEPT tell the caller to use a separate object as the object instead... but they can change every field inside (that's not readonly).

    Of course. It's been a while since I've needed to be mindful of the differences between pass by value and pass by reference. Plus I'm more used to standard OOP practices.



  • @Anonymous Cowherd said:

    @Nexzus said:
    Am I the only one who's deathly afraid of something like 'ref ReturnCodeObject result'?
    No.  But what you're afraid of is what actually happens to 'MyDomainObject', the error state of which is used to control program flow.  (Given the codebase this comes from it could quite easily have been both objects that are altered).

    C# (and Java) could really use some const.
    @Anonymous Cowherd said:
    This dev is also very fond of 'out' parameters.

    Python and Go have good reason for not supporting them.



  • FYI, the original code won't compile since System.String doesn't have a textLength property.

    @Bulb said:

    @Anonymous Cowherd said:
    @Nexzus said:
    Am I the only one who's deathly afraid of something like 'ref ReturnCodeObject result'?
    No.  But what you're afraid of is what actually happens to 'MyDomainObject', the error state of which is used to control program flow.  (Given the codebase this comes from it could quite easily have been both objects that are altered).

    C# (and Java) could really use some const.

    Well, Java does have final parameters. As for C#, I 100% agree with you there. There are some ways to get equivalent functionality using interfaces (e.g.) but those are... clunky, at best.

    @Bulb said:

    @Anonymous Cowherd said:
    This dev is also very fond of 'out' parameters.

    Python and Go have good reason for not supporting them.

    C# supports ref and out because it's designed to interop with Win32 code (as well as some useful edge cases like int.TryParse()). The fact that devs misuse these language features is unfortunate, but generally the use of ref and/or out is a good indication of code smell. In particular, ref should almost never be used in standard C#.



  • @Anonymous Cowherd said:

     protected override void ProcessSomething(string filename, long lineNo, MyDomainObject myDomainObject, ref ReturnCodeObject result)
    {
        string originalText = myDomainObject.originalText;
        int textLen = originalText.textLength;
        ....

    Another WTF (or just a copy pasta error): This won't compile. To get the length of originalText you must use originalText.Length, something a 'senior' developer should know....



  • @keigezellig said:

    @Anonymous Cowherd said:

     protected override void ProcessSomething(string filename, long lineNo, MyDomainObject myDomainObject, ref ReturnCodeObject result)
    {
        string originalText = myDomainObject.originalText;
        int textLen = originalText.textLength;
        ....

    Another WTF (or just a copy pasta error): This won't compile. To get the length of originalText you must use originalText.Length, something a 'senior' developer should know....

     

     No, the textLength value is the length of the text without counting any commas .. He has extended the string class...

     



  • @mikedjames said:

    He has extended the string class...

    Not sure if trolling or stupid.



  • @keigezellig said:

    Another WTF (or just a copy pasta error): This won't compile. To get the length of originalText you must use originalText.Length
    I'd love to stick that on him as well, but it was a 'copy -> slightly obfuscate ->paste error' I'm afraid.

     @The_Assimilator said:

    generally the use of ref and/or out is a good indication of code smell. In particular, ref should almost never be used in standard C#.
    Here's some code he wrote 2 months ago ...

    string pathToDriverFile, pathToOutputFile, pathToAnotherKindOfOutputFile, pathToYetAnotherKindOfOutputFile, unusedPointlessNumberAsAString, connectionString, nameOfDatabaseTableToWriteTo;
    bool useXRatherThanY;
    int userId;

    if (!CheckParams(inputObject, out pathToOutputFile, out pathToDriverFile, out pathToAnotherKindOfOutputFile, out pathToYetAnotherKindOfOutputFile,
         out useXRatherThanY, out userId, out unusedPointlessNumberAsAString, out connectionString, out nameOfDatabaseTableToWriteTo))
    {
        DisplayHelpMessage();
        return;
    }

     


  • Discourse touched me in a no-no place

    @Anonymous Cowherd said:

        int matches = 0;
        for (int i = 0; i < textLen; ++i)
        {
            char ch = originalText[i];
            if (ch == ',')
            {
                ++matches;
                break;
            }
        }
        if (matches <= 0)
    I'm intrigued, under what possible set of circumstances does your senior 'dev' think that matches could possibly be negative?



  • @PJH said:

    I'm intrigued, under what possible set of circumstances does your senior 'dev' think that matches could possibly be negative?
     

    The kind of circumstances where he eats most of the special cake.



  • @keigezellig said:

    @Anonymous Cowherd said:

     protected override void ProcessSomething(string filename, long lineNo, MyDomainObject myDomainObject, ref ReturnCodeObject result)
    {
        string originalText = myDomainObject.originalText;
        int textLen = originalText.textLength;
        ....

    Another WTF (or just a copy pasta error): This won't compile. To get the length of originalText you must use originalText.Length, something a 'senior' developer should know....

    @Anonymous Cowherd said:

    This is production code designed and written by a senior dev here.

    Yup, they've managed to get non-compilable code running in production...

    Is there a reason people post responses like this? What is more likely, an obfuscation, redaction or transcription error, or that there are companies out there where developers sit around writing non-functioniong code, unable to get it to run or even compile, and sitting there stumped, waiting for some genius on TDWTF to explain that they're writing syntactically incorrect and illegal code?



  • @grkvlt said:

    Is there a reason people post responses like this? What is more likely, an obfuscation, redaction or transcription error, or that there are companies out there where developers sit around writing non-functioniong code, unable to get it to run or even compile, and sitting there stumped, waiting for some genius on TDWTF to explain that they're writing syntactically incorrect and illegal code?

    Yes: they have the brain activity of a comatose toad.

    People here are too busy trying to be the most pedantic most dickweed possible to actually stop and think, "wait a minute, did what I just type make any fucking sense at all?"



  • @Nexzus said:

    @Sutherlands said:

    ]...and so does this.  Each and every time you send an object into a function in C# or Java, the calling function can do whatever they want with it EXCEPT tell the caller to use a separate object as the object instead... but they can change every field inside (that's not readonly).

    Of course. It's been a while since I've needed to be mindful of the differences between pass by value and pass by reference. Plus I'm more used to standard OOP practices.

    Translation: I forgot how to program, so now, any time I need memory, I just create a heap object and then abandon it later.



  • @Sutherlands said:

    @Nexzus said:

    Am I the only one who's deathly afraid of something like 'ref ReturnCodeObject result'?

    It does make me think that the developer doesn't understand what they're doing...@Nexzus said:
    Reminds me so much of Win32. "Stick your object into this black hole of a function. We promise we'll take care of it!"
    ...and so does this.  Each and every time you send an object into a function in C# or Java, the calling function can do whatever they want with it EXCEPT tell the caller to use a separate object as the object instead... but they can change every field inside (that's not readonly).

    Great. A couple of chumps on WorseThanFailure.com have decided reference parameters are bad. Let me just go bury my computer in the back yard and start over



  • @Bulb said:

    @Anonymous Cowherd said:
    This dev is also very fond of 'out' parameters.

    Python and Go have good reason for not supporting them.

    Proper multiple parameter returning?



  • @MiffTheFox said:

    @Bulb said:
    @Anonymous Cowherd said:
    This dev is also very fond of 'out' parameters.

    Python and Go have good reason for not supporting them.

    Proper multiple parameter returning?


Log in to reply