|
If rewritten properly this method is tail-recursive and thus ipso facto iterative (by nature).
|
|
|
|
|
Not so long ago, I came across this:
public class StringUtils
{
public bool IsNullOrEmpty(string s)
{
return string.IsNullOrEmpty(s);
}
}
What a gem, hey? I don't know whether to laugh or cry...
|
|
|
|
|
Lol, awesome. Though I can imagine at least a few reasons why this would sort of make sense.
- If somebody moved all the string handling functions to a single class so they don't have to remember the location of all of them, that would make some sense. Not much, but some.
- If somebody was working with a tool that requires a function be exposed by a custom class in order to make use of it. For example, I work with an open source tool called Umbraco, and it makes heavy use of XSLT. In order to use a built-in .Net Framework function from XSLT, I have to create a wrapper function and change some configuration settings to let the XSLT know which DLL to use.
- IsNullOrEmpty was new to .Net Framework 2.0. If they were using a version of .Net before 2.0, they may have created their own implementation, then replaced it with the .Net version.
|
|
|
|
|
Option #1: If I were the boss and this was the case, the guy who did this would be gone (if I could find out who it was, that is)
Option #2: Pretty sure that's not the case here; it is actually a class inside a framework used for building other apps.
Option #3: Perhaps yes. I suppose this could explain it. Although as far as I am aware, I believe it was originally written in .NET 2 already. Maybe the guy came from using .NET 1 and didn't realise this was there in 2.0. :-s LOL.
|
|
|
|
|
Option 1: If you can't find out who did it, the code is the least of your problems. Has your boss ever heard of source control
|
|
|
|
|
LOL. It's a HUGE company and the code is from a different team and from a couple years back... not to mention everyone where I live is named "Nguyen", so I wouldn't have a clue. Hehe
|
|
|
|
|
The simple idea of making this kind of stuff is simply horrid, but the fact that this horrid method is not static is still more horrifying.
|
|
|
|
|
LOL. Actually, my bad; I was quickly writing it up out of memory - I do believe it was static. I dont think anyone can be that idiotic. If I find someone like that, I'm definitely packing my bags and moving on to other things!
|
|
|
|
|
gordon_matt wrote: I dont think anyone can be that idiotic.
Be careful when you say that. Code is like the Darwin Awards... You never know when some moron with a PhD comes up with what they "think" is a great idea...
I wasn't, now I am, then I won't be anymore.
|
|
|
|
|
in that case, could it be that it was written like this:
public static class StringUtils
{
public static bool IsNullOrEmpty(this string s)
{
return string.IsNullOrEmpty(s);
}
}
then you can simplify the calling code a bit:
string a = "11";
string b = "";
string c = null;
Console.Out.WriteLine("a -> {0}, b -> {1}, c -> {2}", a.IsNullOrEmpty(), b.IsNullOrEmpty(), c.IsNullOrEmpty());
instead of
Console.Out.WriteLine("a -> {0}, b -> {1}, c -> {2}", string.IsNullOrEmpty(a), string.IsNullOrEmpty(b), string.IsNullOrEmpty(c));
I wouldn't argue if this is a bad practice or not, but it makes some sense at least (ignoring the issue of calling a seemingly instance method on null reference)
|
|
|
|
|
No; it was definitely NOT an extension method. Even that would make some sense, but this was just completely pointless...
|
|
|
|
|
As it turns out, the company was using this framework for building apps for a particular customer, but that wasn't good enough, they wanted to sell the framework as well, so basically the devs were told to beef up the package a bit... *speechless*
This certainly explains a few oddities in this "Framework"...
Moral of the story: if you're going to outsource, get a dev to review the code before paying for anything. LOL & shaking head at same time...
|
|
|
|
|
Well, that does explain it. Design by upper management strikes again!
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
LOL
it made me laugh enough.
|
|
|
|
|
Everything about this method (PHP) is just wrong... it also throws a fatal error because it's setting $res to a string, and then calls getNext() on it the next iteration. Wtf...
static function searchCausalLink( $pageName, $rev ) {
if ( $rev == 0 ) {
return 'none';
} else {
$res = utils::getSemanticQuery( '[[Patch:+]][[onPage::' . $pageName . ']][[Rev::' . $rev . ']]', '?PatchID' );
if ( $res === false ) return 'none';
$count1 = $res->getCount();
for ( $j = 0; $j < $count1; $j++ ) {
$row1 = $res->getNext();
if ( $row1 === false )
break;
$row1 = $row1[0];
$col1 = $row1->getContent();
foreach ( $col1 as $object1 ) {
$wikiValue1 = $object1->getWikiValue();
$res = $wikiValue1;
}
}
return $res;
}
}
|
|
|
|
|
Academic people apparently don't know about it. This is the second time I find something like this in code written by academics
public function getText() {
$textImage = "";
$tmp = $this->lineList;
$nb = 0;
$nb = sizeof( $tmp );
for ( $i = 1; $i <= $nb; $i++ ) {
if ( $i == 1 ) $textImage = $tmp[$i];
else $textImage = $textImage . "\n" . $tmp[$i];
}
return $textImage;
}
|
|
|
|
|
An inexperienced developer I use to work with kept telling me that she, "..wanted to add exception handling.." to her code. For various reasons, I wasn't quite sure what she meant by that and for other reasons, I didn't bother asking...
A couple weeks later, I did a diff on one of her check-ins to find that she had gone into a particular class (C#) and added try/catch blocks to every function in the class that looked like this:
public void FunctionName()
{
try
{
}
catch (Exception e)
{
throw e
}
}
|
|
|
|
|
Meh, catching and re-throwing exceptions does have its uses, just not with throw e .
Say if you want to log the exception and don't trust that whoever wrote the calling function will do it, you could write;
try
{
}
catch(Exception ex)
{
throw;
}
Or if you catch multiple exceptions and want to re-throw those you don't want to handle like;
try
{
}
catch(SQLException sqlex)
{
}
catch
{
throw;
}
What this actually is, is a chance for you to mentor an inexperienced developer on the correct way to use try/catch blocks.
People are more violently opposed to fur than leather because it's safer to harass rich women than motorcycle gangs
|
|
|
|
|
Rod Kemp wrote: What this actually is, is a chance for you to mentor an inexperienced developer on the correct way to use try/catch blocks.
...and a code horror.
|
|
|
|
|
aspdotnetdev wrote: and a code horror
Personally I would only view this as a true horror if it was done by an experienced developer who should know better, such as that use of Goto that DD shared the other day.
This is a rookie developer that according to the OP kept telling me that she, "..wanted to add exception handling.." to her code which would suggest she didn't know where to start and didn't get any help/tips/pointers on how to go about it.
I think a good way to judge if something is a coding horror is the 2x4 test. If you take into account the experience of the developer and look at the code and think that maybe you should explain how something works, then it's not a horror as such, if however your first thought is, where is that 2x4 so you can beat them over the head with it, then it is a horror.
People are more violently opposed to fur than leather because it's safer to harass rich women than motorcycle gangs
|
|
|
|
|
|
Clearly there are uses for catching an exception of a particular type or of any type, doing some processing and then re-throwing the exception. However, that was not the case here. She just re-threw it. I recognize that it is technically harmless but this individual was supposedly weeks away from graduating with a CS degree - at that point you should understand exceptions, scope and all sorts of fundamentals that this kind of coding shows she does not.
|
|
|
|
|
|
doja93 wrote: I recognize that it is technically harmless
Not really... rethrowing the exception with throw e; breaks the stack and you never get to see where the original exception was thrown.
That said, I'm with what the others said earlier in the thread: It's a good opportunity to mentor someone before they get ruined by some jackaninny who doesn't understand proper exception handling.
=============================
I'm a developer, he's a developer, she's a developer, Wouldn't ya like to be a developer too?
|
|
|
|
|
I'd also like to point out that of all the post I read in this thread, the parent to this post is the only one that was actually rethrowing the exception. Everyone else was repackaging the exception then throwing the repackaged exception which usually has the unintended consequence of losing the stack on the throw. Using the method the parent used ("throw" without a exception type reference) rethrows the last exception, full stack spool and all.
i.e.
Good:
try
{
}
catch(Exception ex)
{
throw;
}
Bad:
try
{
}
catch(Exception ex)
{
throw ex;
}
|
|
|
|