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:
Or just do away with the function altogether.This is not even a conversion. The data types are both string.
Might as well just do this
return pstrFileName;
-
@bstorer said:
@Manos said:
This is not even a conversion. The data types are both string.
Might as well just do this
return pstrFileName;
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:
<font face="arial,helvetica,sans-serif">That's funny, I figured you meant this:</font>@Aaron said:
Yes, that's why I thought it might be a sloppy conversion from VB.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?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>
-
@drachenstern 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.@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>
-
@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)?
-
@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:
Exactly, except VB doesn't do what the OP said, which looks like this: (hence my repost)@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 FunctionFunction FooBar()
variant FooBar
'code
'code
FooBar = someReturnValue
End Function
-
@drachenstern said:
Exactly, except VB doesn't do what the OP said, which looks like this: (hence my repost)
You realize that the OP's code wasn't VB, right?Function FooBar()
variant FooBar
'code
'code
FooBar = someReturnValue
End Function
-
@drachenstern said:
@dhromed said:
Exactly, except VB doesn't do what the OP said, which looks like this: (hence my repost)@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 FunctionFunction FooBar()
variant FooBar
'code
'code
FooBar = someReturnValue
End FunctionBut 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:
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?Exactly, except VB doesn't do what the OP said, which looks like this: (hence my repost)
You realize that the OP's code wasn't VB, right?Function FooBar()
variant FooBar
'code
'code
FooBar = someReturnValue
End Functionpublic 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
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...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.
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.
-
@drachenstern said:
@boomzilla said:
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?
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?You realize that the OP's code wasn't VB, right?
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
@drachenstern said:
@Ilya Ehrenburg said:
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.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...@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 FunctionI 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:
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.I'm not sure what's worse - thinking that an assignment could cause an exception,
-
@DaveK said:
@Aaron 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.I'm not sure what's worse - thinking that an assignment could cause an exception,
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.
-
@indrora said:
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.@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.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.
-
@boomzilla said:
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#.@indrora said:
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.@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.
-
@bjolling said:
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#.@boomzilla said:
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#.@indrora said:
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.@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.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:
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: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?
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 FunctionI 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.