Assignment Exception?



  • I found this today (C# code from vendor app)

    public string GetRptFileName(string pstrFileName)
    {
        string GetRptFileName;
        try
        {
            GetRptFileName = pstrFileName;
        }
        catch (Exception exception1)
        {
            ProjectData.SetProjectError(exception1);
            Exception Err = exception1;
            throw new Exception(Err.Message);
            ProjectData.ClearProjectError();
        }
        return GetRptFileName;
    }
    


  • The way it uses a local variable with a name identical to the function name as well as what appear to be global static method calls makes me think that this is some very poorly-executed (possibly custom-built) automated VB6-to-C# conversion.

    I'm not sure what's worse - thinking that an assignment could cause an exception, throwing a new exception in the catch clause instead of wrapping the caught one, or writing cleanup code immediately after a throw such that the code is guaranteed never to execute.



  • Actually, what's worse is that the re-thrown new exception would be caught by the main program, which calls a static error handling function, which puts up a MessageBox to show the error to the user - but this is a server program, started by a windows service, and nobody is logged in to the server... so the program hangs, and eventually our reporting server will have 50 or more instances of this monster running as the reps try desperately to get their reports, which they will never get. Which I why I was looking at this in the first place. I have been working with this code for about a year now, and this is one of the better WTFs... most of the time, the WTF is buried in spaghetti, and hard to find, and doesn't make for a nice short forum post :)

    Bonus WTF: It works on the vendor's machine!



  • @jasmine2501 said:

    Bonus WTF: It works on the vendor's machine!
    Actually that's called SOP. If it didn't work on theirs either I might call wtfery, but then again, that too would be practically SOP.



  • @Aaron said:

    The way it uses a local variable with a name identical to the function name[...]

     

    I may be wrong, but isn't that just VB's equivalent of a [code]return[/code] statement?



  • This is not even a conversion. The data types are both string.

    Might as well just do this

    return pstrFileName;

     



  • @Manos said:

    This is not even a conversion. The data types are both string.

    Might as well just do this

    return pstrFileName;

    Or just do away with the function altogether.


  • @bstorer said:

    @Manos said:

    This is not even a conversion. The data types are both string.

    Might as well just do this

    return pstrFileName;

    Or just do away with the function altogether.

    Getting rid of the function will just confuse them and cause havoc in the project.

    "Here is that function that returns the string that I pass as a parameter ?"

    Such a change in the application interfaces will require at least 6 months work :-)



  • I only know VB6, I've never programmed in C#, and even I know this function is at best, useless, and at worst, a problem.



  • @PSWorx said:

    @Aaron said:

    The way it uses a local variable with a name identical to the function name[...]

     

    I may be wrong, but isn't that just VB's equivalent of a <font size="2" face="Lucida Console">return</font> statement?

     

    Yes, that's why I thought it might be a sloppy conversion from VB.



  • @Aaron said:

    @PSWorx said:
    @Aaron said:
    The way it uses a local variable with a name identical to the function name[...]
    I may be wrong, but isn't that just VB's equivalent of a <font face="Lucida Console" size="2">return</font> statement?
    Yes, that's why I thought it might be a sloppy conversion from VB.
    <font face="arial,helvetica,sans-serif">That's funny, I figured you meant this:</font>

    public string GetRptFileName(string pstrFileName)
    {
    string GetRptFileName;
    //lots of code
    return GetRptFileName;
    }
    <font face="arial,helvetica,sans-serif">Which I've never done in VB, since VB would look like this:</font>
    public function GetRptFileName(String pstrFileName) as String 
    <font face="arial,helvetica,sans-serif">and C# has also already declared the function name as a string, so I'm confused why you have to declare the String in the body?</font> 

  • ♿ (Parody)

    @drachenstern said:

    @Aaron said:

    Yes, that's why I thought it might be a sloppy conversion from VB.
    <font face="arial,helvetica,sans-serif">That's funny, I figured you meant this:</font>

    public string GetRptFileName(string pstrFileName)
    {
    string GetRptFileName;
    //lots of code
    return GetRptFileName;
    }
    <font face="arial,helvetica,sans-serif">Which I've never done in VB, since VB would look like this:</font>
    public function GetRptFileName(String pstrFileName) as String 
    <font face="arial,helvetica,sans-serif">and C# has also already declared the function name as a string, so I'm confused why you have to declare the String in the body</font>
    That's why he said it was a sloppy conversion, possibly automated, which seems like a reasonable guess to me.  I'm confused why you asked this.


  • @boomzilla said:

    That's why he said it was a sloppy conversion, possibly automated, which seems like a reasonable guess to me.  I'm confused why you asked this.
    Because it didn't seem like what he meant to me when I read it (lack of coffee?) and it still doesn't seem like something a converter would do. Why make a rule to declare a variable internally that mimics the type and name of the function?

    Or did I miss something huge in VB6(and lower)?


  • ♿ (Parody)

    @drachenstern said:

    Why make a rule to declare a variable internally that mimics the type and name of the function?
    Because the VB6 code will have assignments in it to a variable of that name, so adding a variable with the same name and type, and then returning that variable at any exit point is probably the most trivial way to convert from VB6 to C#.



  • @drachenstern said:

    Why make a rule to declare a variable internally that mimics the type and name of the function?
     

    Because that's how VB syntax expresses returns.

    Function FooBar()
       'code
       'code
       'code
       FooBar = someReturnValue
    End Function


  • @dhromed said:

    @drachenstern said:

    Why make a rule to declare a variable internally that mimics the type and name of the function?
     

    Because that's how VB syntax expresses returns.

    Function FooBar()
       'code
       'code
       'code
       FooBar = someReturnValue
    End Function
    Exactly, except VB doesn't do what the OP said, which looks like this: (hence my repost)
    Function FooBar()
       variant FooBar
       'code
       'code
       FooBar = someReturnValue
    End Function


  • ♿ (Parody)

    @drachenstern said:

    Exactly, except VB doesn't do what the OP said, which looks like this: (hence my repost)

    Function FooBar()
       variant FooBar
       'code
       'code
       FooBar = someReturnValue
    End Function
    You realize that the OP's code wasn't VB, right?


  • @drachenstern said:

    @dhromed said:

    @drachenstern said:

    Why make a rule to declare a variable internally that mimics the type and name of the function?
     

    Because that's how VB syntax expresses returns.

    Function FooBar()
       'code
       'code
       'code
       FooBar = someReturnValue
    End Function
    Exactly, except VB doesn't do what the OP said, which looks like this: (hence my repost)
    Function FooBar()
       variant FooBar
       'code
       'code
       FooBar = someReturnValue
    End Function


     

    But if you wanted to write a sloppy converter, wouldn't the natural thing to do be replacing

    Function FooBar() As Thing

    with

    Thing FooBar()

    {

        Thing FooBar;


    and

    End Function

    with


        return FooBar;

    }


    so you needn't worry about the assignments in the function body? It would even work on your simple test cases.



  • @boomzilla said:

    @drachenstern said:
    Exactly, except VB doesn't do what the OP said, which looks like this: (hence my repost)

    Function FooBar()
       variant FooBar
       'code
       'code
       FooBar = someReturnValue
    End Function

    You realize that the OP's code wasn't VB, right?
    you realize the OPs code looked virtually the same, right? Hence my point. If the original function is typed, then why declare a variable inside the function with the same name and type as the function?

    public string GetRptFileName(string pstrFileName)
    {
    string GetRptFileName;
    return GetRptFileName;
    }

    Hell, I would expect a compiler to burp on that one, even if it is legal (which I doubt). As to

    @Ilya Ehrenburg said:

    But if you wanted to write a sloppy converter, wouldn't the natural thing to do be replacing

    Function FooBar() As Thing

    With

    Thing FooBar()
    {
        Thing FooBar;

    and

    End Function

    with

        return FooBar;
    }

    so you needn't worry about the assignments in the function body? It would even work on your simple test cases.

    No, I wouldn't expect to see that, because it's just too farfetched to see the interpreter grab the function type and recast the function name as the type within the function. And again, I doubt that's legal language construction according to a grammar, even if it is compilable. Which just goes to show, I don't write compilers. That and the OP code was C#, and I've never written C# code like that...

    I would expect to see "End Function" replaced with "}", because not all functions will return a value. Or are you assuming the interpreter (which is already abhorrently designed enough that it does the first thing) will not account for the second set of behaviours? Granted, it probably would/could/does, but we don't know that, and I'm assuming a lazy interpreter (as you pointed out) so the laziest thing to do is what you described, but not the best way to write a converter.



  • Actually the whole thing is dumb, since all it can ever do is return the same thing that was sent in...

    Which means somewhere in the calling program, it is doing:

    myString = myString;
    

    Which is, again, WTFery.


  • ♿ (Parody)

    @drachenstern said:

    @boomzilla said:

    You realize that the OP's code wasn't VB, right?

    you realize the OPs code looked virtually the same, right? Hence my point. If the original function is typed, then why declare a variable inside the function with the same name and type as the function?

    public string GetRptFileName(string pstrFileName)
    {
    string GetRptFileName;
    return GetRptFileName;
    }

    Hell, I would expect a compiler to burp on that one, even if it is legal (which I doubt). As to

    Yes, they looked almost the same, which is why it looks like an automatic translation.  The original function is typed, so the translator knows what type to use.  Why wouldn't this be legal?

    @drachenstern said:

    @Ilya Ehrenburg said:
    so you needn't worry about the assignments in the function body? It would even work on your simple test cases.

    No, I wouldn't expect to see that, because it's just too farfetched to
    see the interpreter grab the function type and recast the function name
    as the type within the function. And again, I doubt that's legal
    language construction according to a grammar, even if it is compilable. Which just goes to show, I don't write compilers. That and the OP code was C#, and I've never written C# code like that...
    WTF are you talking about here?  What is far fetched about any of this?  It has nothing to do with writing compilers.  Writing code should be enough to understand it.

    @drachenstern said:

    I would expect to see "End Function" replaced with "}", because not all functions will return a value. Or are you assuming the interpreter (which is already abhorrently designed enough that it does the first thing) will not account for the second set of behaviours? Granted, it probably would/could/does, but we don't know that, and I'm assuming a lazy interpreter (as you pointed out) so the laziest thing to do is what you described, but not the best way to write a converter.
    But this function obviously did return a value.  Are you being purposefully obstinate about this?  No one said this was a great thing to do.  In fact, the guy who mentioned it originally talked about it as a 'sloppy' converter.  And when has this site been about the best anything?

     

     



  • @drachenstern said:

    Exactly, except VB doesn't do what the OP said, which looks like this: (hence my repost)
    Function FooBar()
       variant FooBar
       'code
       'code
       FooBar = someReturnValue
    End Function

     

    I really didn't think that this would be so complicated.  The problem with an automated conversion is that the "return" statement in C/C++/C# actually terminates execution of the function, whereas in VB it just sets the result while allowing execution to continue.

    Look, let's say you have some sloppily-written VB code that looks like this:

    Function GetFoo(ByVal x As Integer) As Integer
        Dim multiply As Boolean
        If x > 0 Then
            GetFoo = 1
        Else
            GetFoo = -1
        End If
        ... A bunch of code that sets "multiply" ...
        If multiply Then GetFoo = GetFoo * 100
    End Function
    Now what's the easiest way to convert this to C#? You can't just replace every "GetFoo" with a "return", because half of the code will never get executed. So instead you have to use a temporary variable that behaves like the "GetFoo" pseudo-variable in VB. What's the best name for it? Well, probably the same name as the function:
    int GetFoo(int x)
    {
      int _GetFoo;
      bool multiply;
      if (x > 0)
      {
        _GetFoo = 1;
      }
      else
      {
        _GetFoo = -1;
      }
      ... A bunch of converted code ...
      if (multiply)
      {
        _GetFoo = _GetFoo * 100;
      }
      return _GetFoo;
    }

    I added an underscore, but that's obviously a matter of preference. Maybe somebody wrote a converter that uses the identical name.

    Get it now?



  • @Aaron said:

    Get it now?
    Actually, yeah, that made a lot of sense, because I didn't think you could do that...



  • @Aaron said:

    I'm not sure what's worse - thinking that an assignment could cause an exception, 

    Does C# do ref-counted or CoW strings?  In C++ I wouldn't see why that might not run out of memory in the copy c-tor and throw. 



  • @DaveK said:

    @Aaron said:

    I'm not sure what's worse - thinking that an assignment could cause an exception, 

    Does C# do ref-counted or CoW strings?  In C++ I wouldn't see why that might not run out of memory in the copy c-tor and throw. 

    There's no such thing as a pointer in C# unless you use some really unsafe code.

    if C# doesnt like it, the compiler catches it pretty quickly. Not saying this is a reason to make stupid code and rely on the compiler, but things like implicitly turning a string into an int is caught in the IL conversion. This really gets ignored by the compiler and turns into IL that looks like "push,ret" insteand of "push,ldstr.0,cmp lderr.0 null " (with a lot of useless crap for jmps and such goat-blowery. ) because it sees an assignment between two like types, and just simplifies the try clause, if i recall correctly. I'd have to see the compiled IL for this.


  • ♿ (Parody)

    @indrora said:

    @DaveK said:

    Does C# do ref-counted or CoW strings?  In C++ I wouldn't see why that might not run out of memory in the copy c-tor and throw.
    There's no such thing as a pointer in C# unless you use some really unsafe code.

    Huh?  MS' CLR uses pointers all over the place, any time you have a reference type object.  It's generally all transparent to C#, but that doesn't mean they're not there, including for strings.

    Strings are immutable, so I'd imagine that this assignment would simply put the pointer to the orignial string in the slot for that variable and let the garbage collector sort it all out.

    The only time I could imagine an exception being thrown would be if it were actually a property using a set method that got clever, which is obviously not happening here.


  • :belt_onion:

    @boomzilla said:

    @indrora said:

    @DaveK said:

    Does C# do ref-counted or CoW strings?  In C++ I wouldn't see why that might not run out of memory in the copy c-tor and throw.
    There's no such thing as a pointer in C# unless you use some really unsafe code.

    Huh?  MS' CLR uses pointers all over the place, any time you have a reference type object.  It's generally all transparent to C#, but that doesn't mean they're not there, including for strings.

    Of course the CLR can use pointers because it isn't written in C#. They are not transparant to C#, they are simple not there. The way the runtime handles strings (with or without pointers) has no bearing on C#.

  • ♿ (Parody)

    @bjolling said:

    @boomzilla said:

    @indrora said:

    @DaveK said:

    Does C# do ref-counted or CoW strings?  In C++ I wouldn't see why that might not run out of memory in the copy c-tor and throw.
    There's no such thing as a pointer in C# unless you use some really unsafe code.

    Huh?  MS' CLR uses pointers all over the place, any time you have a reference type object.  It's generally all transparent to C#, but that doesn't mean they're not there, including for strings.

    Of course the CLR can use pointers because it isn't written in C#. They are not transparant to C#, they are simple not there. The way the runtime handles strings (with or without pointers) has no bearing on C#.
    Yeah, I wasn't really clear about this.  DaveK was talking about how strings were implemented, and then indrora wrote a non-sequitor about pointers and C#.

    I wouldn't go so far as to say that they aren't there, we just don't really talk about them as such, but any reference object is really a reference (pointer!) to the object, and when passed, that reference is passed by value.  It's mostly transparent, but can come back to bite you in some cases, if you do certain modifications, unless you declare the parameter as 'ref' (and pass it that way, too). 

    A pointer by any other name...



  • @drachenstern said:

    @Aaron said:

    Get it now?
    Actually, yeah, that made a lot of sense, because I didn't think you could do that...

     

    Have you ever programmed in any static and strong typed language? (C, C++, C#, Java for example? There is no reason why you couldn't do that: You just declare a variable with the same type as the return value in the function scope and return that variable somewhere in the function.

    Whatever was done with i doesn't even matter, since you know that the function returns that type and thanks to the strong typing of the language you are guaranteed that that variable is a valid return value.

    Why did you think it couldn't be allowed?



  • No offense taken or presumed, because ...@dtech said:

    @drachenstern said:
    Actually, yeah, that made a lot of sense, because I didn't think you could do that...
    Have you ever programmed in any static and strong typed language? (C, C++, C#, Java for example? There is no reason why you couldn't do that: You just declare a variable with the same type as the return value in the function scope and return that variable somewhere in the function.

    Whatever was done with it doesn't even matter, since you know that the function returns that type and thanks to the strong typing of the language you are guaranteed that that variable is a valid return value.

    Why did you think it couldn't be allowed?

    All I've ever heard or read about languages says that casting two variables with the same name within the same scopeis illegal. For instance, in C, if I try:

    int main(){
      int main;
      return 0;
    }

    I expect it to puke on the internal declaration. By the same notion, I expect the following to puke because of the second declaration:

    int function_whatever() {
      int pfX;
      pfX = 0;
      pfX = function_somethingelse();
      int pfX;
      pfX = 0;
    }

    However, having said that, I also recognize that scoping has limits, so for instance declaring `for (int itr = 0; itr<MAX_ITR; itr++)` or whatever, the itr is supposed to fall out of scope when the for loop exits (if it does or not is a compiler idiosyncracy, but the spec says it falls out of scope). In that case, one could declare itr repeatedly, although I think we all know that declaring what the loop iterator is for in the variable name makes a lot more sense.

    I just didn't realize that one could call a function like so:

    public function strFunc (oper1 as Variant, oper2 as Variant) as String
      Dim strFunc as String
      strFunc = oper1 + oper2
    End Function

    I figured it would puke on the dim internally. Something about scope of declarations and valid naming. seems like either the internal value disappears when you leave (because the scope has ended before you do something with it) or else it would cause a compiler error, thus not being acceptable grammar. And because I didn't know it was valid, I figured it couldn't be done.

    Granted, I can't see that coding like that would be more or less beneficial in a strongly typed language, so I wouldn't have tried it in the first place. If you've already declared it within the scope, why declare it again and again? In case it didn't get declared?



  • @drachenstern said:

    All I've ever heard or read about languages says that casting two variables with the same name within the same scopeis illegal.
    Two variables within the same scope, yes.  But two variables in different scopes which overlap is okay, as is a variable and a function.@drachenstern said:

    For instance, in C, if I try:

    int main(){
      int main;
      return 0;
    }

    I expect it to puke on the internal declaration.

     

    $ cat main.c
    #include "stdio.h"

    int main () {
      int main;
      main = 3;
      printf("%d", main);
      return 0;
    }

    $ gcc -ansi -pedantic -Wall -W -Wshadow main.c -o main.out
    main.c: In function `main':
    main.c:4: warning: 'main' is usually a function
    main.c:4: warning: declaration of 'main' shadows a global declaration
    main.c:3: warning: shadowed declaration is here

    $ ./main.out
    3




  • So... apparently the story behind this is pretty simple. The function used to strip the file name out from the full path plus filename string. So, when the input parameter changed from being the full path to being just the filename, some genius went in a basically no-opped this function, rather than re-factoring the code to eliminate the calls to it. So it's eating up clock cycles and memory, doesn't do anything, but is called in many places. Still WTF, because there is a built-in .Net function to do what this one used to do - but then, this is the company that built its own eMail sender library too, rather than use the .Net one.

    For those who wanted to see the IL:

    .method public instance string GetRptFileName(string pstrFileName) cil managed
    {
        .maxstack 2
        .locals init (
            [0] string GetRptFileName,
            [1] class [mscorlib]System.Exception Err)
        L_0000: nop 
        L_0001: nop 
        L_0002: ldarg.1 
        L_0003: stloc.0 
        L_0004: leave.s L_0021
        L_0006: dup 
        L_0007: call void [Microsoft.VisualBasic]Microsoft.VisualBasic.CompilerServices.ProjectData::SetProjectError(class [mscorlib]System.Exception)
        L_000c: stloc.1 
        L_000d: nop 
        L_000e: ldloc.1 
        L_000f: callvirt instance string [mscorlib]System.Exception::get_Message()
        L_0014: newobj instance void [mscorlib]System.Exception::.ctor(string)
        L_0019: throw 
        L_001a: call void [Microsoft.VisualBasic]Microsoft.VisualBasic.CompilerServices.ProjectData::ClearProjectError()
        L_001f: leave.s L_0021
        L_0021: nop 
        L_0022: ldloc.0 
        L_0023: ret 
        .try L_0002 to L_0006 catch [mscorlib]System.Exception handler L_0006 to L_0021
    }
    
    
    

    Looks like it is converted from VB.



  • Looks like it is converted from VB

    [code]L_0007[/code] gives it away any day. usually, things being converted from VB->C# are horrible in that fashion.



  • @jasmine2501 said:

    but then, this is the company that built its own eMail sender library too, rather than use the .Net one.

    While the .NET email library (System.Net.Mail) is good, there is no complete control over the resulting MIME structure and the system just lacks extensibility. You cannot use different transfer encodings than 7-bit, quoted-printable and base64; 8-bit is missing. Also the quoted-printable encoding is buggy.



  • Well, that would make sense if the email system in question were capable of sending anything other than plain text. I have used the .Net Mail to send HTML newsletters with no issue. Anything more complicated than that just gets blocked by spam filters anyway.


Log in to reply