Comment On Better Off TODO

Maybe none of the parts have descendents.  So really, it's more
efficient to simply return false than search on something you know
isn't there.

It's an optimization. The compiler can inline the return false which is much faster than doing a Regex.IsMatch.

Oh, I get it now... he's wasting all the clock cycles concatenating strings. He should wrap that in an if(logLevel == LogLevel.DEBUG) block.

Is strAncestorID a member variable, or was it actually supposed to be intAncestorID(the passed parameter)?



So, we're probably seeing that PartTree is actually a comma delimited
list of some sort?  Gotta see more code to see the level of true
WTFery.

ahhhh, it took me some time to understand

the code snippets in this site are not belonged to bad programmers

they simply want to insure their job is for a life time :)

http://thc.org/root/phun/unmaintain.html

have fun

Ytram:

Is strAncestorID a member variable, or was it actually supposed to be intAncestorID(the passed parameter)?

I was wondering that myself. It doesn't really makes sense to call ToString() on a string variable, so one would assume it was suppose to be a different data type. Although, the passed in variable was intAncesterNum, not intAncesterID. Maybe it's a typo; otherwise, there is some kind of disconnect between the passed in variable and the rest of the function that I don't quite understand.

 

Well to be honest it looks like a case of someone biting off more than they could chew....

first try:

string strAncestor = strAncestorID.ToString();

if ((regex.ismatch(this.PartTree,strAncestor)....

... of course when they ran this with a AncestorID of 1 they got matches for "1", "10","11","21","100","231" and anything else that contained a "1"

so programmer slaps forehead and further specifies the condition

second try:

string strAncestor = ","+strAncestorID.ToString()+",";

if ((regex.IsMatch(this.PartTree,strAncestor)....

... this time they only got matches for partID "1" except that they missed it if partID "1" was a the beginning or the end of the part list.

so programmer says 'arghhhh!' and starts doing the whole start, end, middle thing but happens to think for a moment and see that now the regex will return matches for ",1," correctly in the middle and "1," correctly at the beginning and ",1" correctly at the end, but will also return matchs for ",201," incorrectly by applying the beginning rule.  The orginial programmer then says 'dang!' comments out the function portion of the code, returns false and forgets about the whole idea.

This is completely OT but Buckaroo Bonzai was on this weekend and watched part of it.  Made me think of John Smallberries.  I should have been John Bigboote.

Eighth!

I'm registered!

Anonymous:

Well to be honest it looks like a case of someone biting off more than they could chew....

first try:

string strAncestor = strAncestorID.ToString();

if ((regex.ismatch(this.PartTree,strAncestor)....

... of course when they ran this with a AncestorID of 1 they got matches for "1", "10","11","21","100","231" and anything else that contained a "1"

so programmer slaps forehead and further specifies the condition

second try:

string strAncestor = ","+strAncestorID.ToString()+",";

if ((regex.IsMatch(this.PartTree,strAncestor)....

... this time they only got matches for partID "1" except that they missed it if partID "1" was a the beginning or the end of the part list.

so programmer says 'arghhhh!' and starts doing the whole start, end, middle thing but happens to think for a moment and see that now the regex will return matches for ",1," correctly in the middle and "1," correctly at the beginning and ",1" correctly at the end, but will also return matchs for ",201," incorrectly by applying the beginning rule.  The orginial programmer then says 'dang!' comments out the function portion of the code, returns false and forgets about the whole idea.

Yeah, your theory fits the data very well. Looks like someone in over their head having difficulties. Not nearly as much as a WTF as the for-switch paradigm or the switch with a default case only. We've all had similar troubles and made embarrassing gaffes.

JohnO:

This is completely OT but Buckaroo Bonzai was on
this weekend and watched part of it.  Made me think of John
Smallberries.  I should have been John Bigboote.

Too late! bwahahaha

This is where the hack ("," + this.partTree + ",").indexOf("," + strAncestor + ",") != -1   is useful.

I think it's not WTF, programmer just was mathematician. He started to
write function and then, in middle of doing he realised there is much
better way... probability. He did some research and found out it will
return false in most cases, so simplyfied the function by always
returning false and added that few lines of strange code to diguise his
genial idea. I think we are looking at one of the most effective
functions ever written.

Anonymous:

ahhhh, it took me some time to understand
the code snippets in this site are not belonged to bad programmers
they simply want to insure their job is for a life time :)
http://thc.org/root/phun/unmaintain.html
have fun

 

nice read ... very helpful!!!

Just another WTF:

Well to be honest it looks like a case of someone biting off more than they could chew....

first try:

string strAncestor = strAncestorID.ToString();

if ((regex.ismatch(this.PartTree,strAncestor)....

... of course when they ran this with a AncestorID of 1 they got matches for "1", "10","11","21","100","231" and anything else that contained a "1"

so programmer slaps forehead and further specifies the condition

second try:

string strAncestor = ","+strAncestorID.ToString()+",";

if ((regex.IsMatch(this.PartTree,strAncestor)....

... this time they only got matches for partID "1" except that they missed it if partID "1" was a the beginning or the end of the part list.

so programmer says 'arghhhh!' and starts doing the whole start, end, middle thing but happens to think for a moment and see that now the regex will return matches for ",1," correctly in the middle and "1," correctly at the beginning and ",1" correctly at the end, but will also return matchs for ",201," incorrectly by applying the beginning rule.  The orginial programmer then says 'dang!' comments out the function portion of the code, returns false and forgets about the whole idea.

I did this once, except I made my comma-delimited list start and end with commas so I could search for ",20," in ",1,2,3,5,20,454,". The on my second day of programming, I learned how to do it right using built-in functions to split the list and search more efficiently.

Manni:

I did this once, except I made my
comma-delimited list start and end with commas so I could search for
",20," in ",1,2,3,5,20,454,". The on my second day of programming, I learned how to do it right using built-in functions to split the list and search more efficiently.

If you are searching the same list again and again, splitting it into
pieces and filling a adaquate container (hashtable or tree) is
definitely faster. On the other hand, if you have to search the list
only once, or if the list is short, the simple solution (searching for
",20," within the string)  is probably faster.

Manni:

I did this once, except I made my comma-delimited list start and end with commas so I could search for ",20," in ",1,2,3,5,20,454,". The on my second day of programming, I learned how to do it right using built-in functions to split the list and search more efficiently.

Define "efficiently".  Your first version probably ran several times faster.

ammoQ:

On the other hand, if you have to search the list only once, or if the list is short, the simple solution (searching for ",20," within the string)  is probably faster.

Ooops... Should have kept reading before I responded......

I can see what the guy was getting at here -- using the matches from
start, middle or end to construct an open ended hierarchical tree and
quickly identify when we're dealing with the head or the foot. 
It's the sort of clever implementation somebody in
a Perl/PHP world might come up with -- regular expressions and string handling being the strong suits of these packages.



But more than anything, it demonstrates poor understanding of software
(and probably database) design.  A set of integers implemented as
a string array?  In code that is (ostensibly) persisted in a
database?  Not only is it slow and wasteful, it's also impossible
to maintain and worthless for reporting.  Back to DeVry, guy.

Anonymous:

I can see what the guy was getting at here -- using the matches from start, middle or end to construct an open ended hierarchical tree and quickly identify when we're dealing with the head or the foot.  It's the sort of clever implementation somebody in a Perl/PHP world might come up with -- regular expressions and string handling being the strong suits of these packages.

But more than anything, it demonstrates poor understanding of software (and probably database) design.  A set of integers implemented as a string array?  In code that is (ostensibly) persisted in a database?  Not only is it slow and wasteful, it's also impossible to maintain and worthless for reporting.  Back to DeVry, guy.

Doing queries from an RDBMs which stores a hierarchy is a somewhat non-trivial task. For example, if you were to provide us a query to display all descendants of a node in a hierarchy modelled by a table which foreign keys into itself, I'm sure many people on this forum would find problems with your solution.

Someone getting in over their head is a minor WTF compared to the beasts we've seen here. Though the real WTF is the comments people on both sides have been making about the metric system. And to think that I once believed that only church-goers participated in holy wars... [A]

 

yaccobb:

I think it's not WTF, programmer just was mathematician. He started to
write function and then, in middle of doing he realised there is much
better way... probability. He did some research and found out it will
return false in most cases, so simplyfied the function by always
returning false and added that few lines of strange code to diguise his
genial idea. I think we are looking at one of the most effective
functions ever written.

I resent that.



What you meant to say is that the programmer was a statistician.  From Purdue.

This should be a TODO, this should be removed completely from the
code.   If this is in some DLL/shared library which is must
remain for binary compatibility (though it looks like Java to me which
I don't think has this problem?) it should throw a fatal exception so
that all cases that call this can be fixed.

Anonymous:

I can see what the guy was getting at here -- using the matches from start, middle or end to construct an open ended hierarchical tree and quickly identify when we're dealing with the head or the foot.  It's the sort of clever implementation somebody in a Perl/PHP world might come up with -- regular expressions and string handling being the strong suits of these packages.

But more than anything, it demonstrates poor understanding of software (and probably database) design.  A set of integers implemented as a string array?  In code that is (ostensibly) persisted in a database?  Not only is it slow and wasteful, it's also impossible to maintain and worthless for reporting.  Back to DeVry, guy.

The really sad part of all this is that the this.PartTree method probably has to walk the structure anyway in order to create its comma seperated list... everytime you call PartTree.  So a function called isDescendant could save a ton of time and effort by simply walking the structure once and returning true when it comes across the integer PartID of the looked for ancestor.  Instead simple core functionality is being implemented by leveraging UI methods... that is they've call PartTree which has to walk the whole tree retrieve the PartId's and build a comma delimited string, then convert the integer parameter into a string and do whatever extra string handling they use to solve their 'partial matches' problem.  Then pass that whole wack of strings into the RegEx engine.  Just imagine all the extra code your carrying, all the extra memory you use, and all those clock cycles that would otherwise have gone to waste.  Its code like this that makes it worthwhile to get the latest and greatest hardware.

this.PartTree doesn't look like it's a method, just a class variable.
It was probably constructed as you described, but only once (or once
for each time it gets updated).

John Bigboote:

JohnO:

This is completely OT but Buckaroo Bonzai was on
this weekend and watched part of it.  Made me think of John
Smallberries.  I should have been John Bigboote.

Too late! bwahahaha

There's always John Yaya...

ammoQ:

If you are searching the same list again and again, splitting it into
pieces and filling a adaquate container (hashtable or tree) is
definitely faster. On the other hand, if you have to search the list
only once, or if the list is short, the simple solution (searching for
",20," within the string)  is probably faster.

Right up to the point that 20 is at the front or the end, so it either doesn't have commas leading or trailing.

Anonymous:

Right up to the point that 20 is at the front or the end, so it either doesn't have commas leading or trailing.

AmmoQ is commenting on Manni statement just before.  Read Manni's again....Closely

Anonymous:

this.PartTree doesn't look like it's a method, just a class variable.
It was probably constructed as you described, but only once (or once
for each time it gets updated).

If this code is C#, then PartTree could be a property, which in turn
means that accessing it could very well be a full-fledged function call
(to the getter function defined for it)...

Manni:

Then on my second day of
programming, I learned how to do it right using built-in functions to
split the list and search more efficiently.

~on the second day of christmas, my true love gave to me...~



KNOWLEDGE

Krenn:

ammoQ:

If you are searching the same list again and again, splitting it into
pieces and filling a adaquate container (hashtable or tree) is
definitely faster. On the other hand, if you have to search the list
only once, or if the list is short, the simple solution (searching for
",20," within the string)  is probably faster.

Right up to the point that 20 is at the front or the end, so it either doesn't have commas leading or trailing.

Ever heard the joke about how to catch an elephant? And how the
programmer makes sure to place a known instance of an elephant in cape
town before iterating over Africa from the north to make sure the
algorithm terminates?



(... which is why you make sure there are commas at the end and
beginning of the string you want to search, by putting them there
yourself.)

...had a lot of advertised "details" that just didn't seem to work...

To be honest, knowing how the average sales+marketing team thinks, I'm not surprised they were advertising features that hadn't been written yet; I've seen it happen more times than I can count, and it usually ends up causing trouble for the development team.

The trick is just not to tell them anything about what you're working on until it has at least passed the first stage of QA. And whatever you do, don't ever show them your mockups or prototypes, because sales people interpret those as completed products, and promptly start trying to sell them.

Anonymous:

...had a lot of advertised "details"
that just didn't seem to work...

To be honest, knowing how the average sales+marketing team thinks, I'm
not surprised they were advertising features that hadn't been written
yet; I've seen it happen more times than I can count, and it usually
ends up causing trouble for the development team.
The trick is just not to tell them anything about what you're working
on until it has at least passed the first stage of QA. And whatever you
do, don't ever show them your mockups or prototypes, because sales
people interpret those as completed products, and promptly start trying
to sell them.

My favourite version of the problem:



Sales team sells stuff yet-to-be-made, with a deadline that couldn't be
met even if you had ten times as many people in your team. To make it
even worse, the specification of the project is vague so it's difficult
to get even complete and accurate specs till the deadline. You start
making mockups and prototypes, so at least you have something to show
and discuss.

Then, when the deadline comes close, the project is postponed for
external reasons (this happens every single time). Management assigns
the team to another (of course also late) project. Six months later,
when the new deadline is due, the old project is revitalized and the
team has few weeks to turn the mockups and prototypes into the real
thing.

Dear Recognition,



Why must you taunt me so?



Love,

DH.

yaccobb:

I think it's not WTF, programmer just was mathematician. He started to
write function and then, in middle of doing he realised there is much
better way... probability. He did some research and found out it will
return false in most cases, so simplyfied the function by always
returning false and added that few lines of strange code to diguise his
genial idea. I think we are looking at one of the most effective
functions ever written.

But what about the bias ???? That's not adressed at all!

OneFactor:

And to think that I once believed that only church-goers participated in holy wars... [A]

I take it that was before you learned programming.

OneFactor:

For example, if you were to provide us a
query to display all descendants of a node in a hierarchy modelled by a
table which foreign keys into itself, I'm sure many people on this
forum would find problems with your solution.

Oracle can do exactly that (the following example is copied from the Oracle docs)

SELECT LPAD(' ',2*(LEVEL-1)) || ename org_chart, empno, mgr, job

    FROM emp

    START WITH job = 'PRESIDENT' 

    CONNECT BY PRIOR empno = mgr;

It's part of the new methodology: Design Driven Development. What UAT
does not catch is not worth paying any attention to and until the
client agrees to a new quote, dont worry!



<tongue-in-cheek />

Alex Papadimoulis:

  string strAncestor = intAncestorNum.ToString();
  string start = strAncestor + ",";
  string end = "," + strAncestor;
  string middle = "," + strAncestor + ",";

I like how strAncestor is named in (Systems) Hungarian (along with
intAncestorNum - but that's a discussion for another day), but this
convention is completely dropped for the next three strings, start,
middle, and end.  Good going!

ammoQ:

Sales team sells stuff yet-to-be-made, with a deadline that couldn't be
met even if you had ten times as many people in your team. To make it
even worse, the specification of the project is vague so it's difficult
to get even complete and accurate specs till the deadline. You start
making mockups and prototypes, so at least you have something to show
and discuss.

Then, when the deadline comes close, the project is postponed for
external reasons (this happens every single time). Management assigns
the team to another (of course also late) project. Six months later,
when the new deadline is due, the old project is revitalized and the
team has few weeks to turn the mockups and prototypes into the real
thing.

Actually, I remember once a guy from the delivery service entering my office and asking about an unknown (by him and me) product he had to send to a customer (who had already paid it).
After several hours of research we discovered someone from the sale team just sold to the customer what he asked for, and noone in the dev team even knew about it. The product didn't exist, the design document didn't exist, the features list didn't exist, ... We had to develop the thing in a few days and I've never seen again a soft with that much bugs (we didn't even have the time to develop an interface).

Alex Papadimoulis:

Peter S. noticed that his
company's flagship product, a parts management system, had a lot of
advertised "details" that just didn't seem to work. Being the motivated
guy that he is, Pete took it upon himself to fix these, starting with a
non-functional search for all container parts given a particular part.
Upon looking at the code, he decided that maybe it was just better off
left TODO ...

/// 


/// returns true if the part is a descendent of the specified part

/// 

public bool IsDescendent(int intAncestorNum)
{
  string strAncestor = intAncestorNum.ToString();
  string start = strAncestor + ",";
  string end = "," + strAncestor;
  string middle = "," + strAncestor + ",";

  // TODO ...

  //if ((Regex.IsMatch(this.PartTree, strAncestor

  return false;
}

(Update: Fixed typo I made in the code)

Is there any reason that the regex "\D?" + intAncestorNum.ToString() + "\D?" wouldn't have worked? WTF?

Anonymous:

Alex Papadimoulis:

Peter S. noticed that his
company's flagship product, a parts management system, had a lot of
advertised "details" that just didn't seem to work. Being the motivated
guy that he is, Pete took it upon himself to fix these, starting with a
non-functional search for all container parts given a particular part.
Upon looking at the code, he decided that maybe it was just better off
left TODO ...

/// 


/// returns true if the part is a descendent of the specified part

/// 

public bool IsDescendent(int intAncestorNum)
{
  string strAncestor = intAncestorNum.ToString();
  string start = strAncestor + ",";
  string end = "," + strAncestor;
  string middle = "," + strAncestor + ",";

  // TODO ...

  //if ((Regex.IsMatch(this.PartTree, strAncestor

  return false;
}

(Update: Fixed typo I made in the code)

Is there any reason that the regex "\D?" + intAncestorNum.ToString() + "\D?" wouldn't have worked? WTF?

Other than the fact that due to the '?' both '\D's are entirely
optional and thus would catch every instance of intAncestorNum no
matter where it was?

Casiotone:

Anonymous:

Is there any reason that the regex "\D?" + intAncestorNum.ToString() + "\D?" wouldn't have worked? WTF?

Other than the fact that due to the '?' both '\D's are entirely
optional and thus would catch every instance of intAncestorNum no
matter where it was?

"\b" + intAncestorNum.ToString() + "\b" should work fine though.

Maurits:

Casiotone:

Anonymous:

Is there any reason that the regex "\D?" + intAncestorNum.ToString() + "\D?" wouldn't have worked? WTF?

Other than the fact that due to the '?' both '\D's are entirely
optional and thus would catch every instance of intAncestorNum no
matter where it was?

"\b" + intAncestorNum.ToString() + "\b" should work fine though.

As would

"(^|,)"+intAncestorNum.toString()+"(,|$)"

(your solution is much cleaner though)

:pizza:

[pi]

Where I work a salesman selling stuff we haven't written yet is a sackable offence.

masklinn:

Maurits:

Casiotone:

Anonymous:

Is there any reason that the regex "\D?" + intAncestorNum.ToString() + "\D?" wouldn't have worked? WTF?

Other than the fact that due to the '?' both '\D's are entirely
optional and thus would catch every instance of intAncestorNum no
matter where it was?

"\b" + intAncestorNum.ToString() + "\b" should work fine though.

As would

"(^|,)"+intAncestorNum.toString()+"(,|$)"

(your solution is much cleaner though)

Yes, in this case they are identical.  The "\b" version is not supported in as broad a selection of regex dialects, so the "(^|,)" version has its merits too.

A major WTF here is that it seems that the developer was going to do 3 regex searches where one would have been fine.  Regex matching is reasonably expensive.  Doing three searches where one is sufficient is usually a bad thing.

On the other hand, if the combined regex is much more complex than the individual regexen, it might be worth doing for maintainability.  Speed could be better if the individual regexen were hand tuned or all possibilities didn't need to be checked each time.

ammoQ:

Manni:

I did this once, except I made my comma-delimited list start and end with commas so I could search for ",20," in ",1,2,3,5,20,454,". The on my second day of programming, I learned how to do it right using built-in functions to split the list and search more efficiently.

If you are searching the same list again and again, splitting it into pieces and filling a adaquate container (hashtable or tree) is definitely faster. On the other hand, if you have to search the list only once, or if the list is short, the simple solution (searching for ",20," within the string)  is probably faster.

Yeah in the case where I was using the comma-delimited string, I had to search it repeatedly to map out items that matched up to an ID column in a database. It worked a little better when I started using tables, considering the code now looks through several hundred (or maybe thousand by this point) unique IDs.

when i was in my first year of studying IT, we had to learn all about
Binary Search Trees and Tries and Hash Tables and 2-3 Trees. Seeing as
i'm still studying that degree and don't have that much real world
experience, I was just wondering, how often are these used? The seem to
be probably a bit more efficient than using strings and regexes,
especially when you start getting into the hundreds or thousands of ids.

Anonymous:

when i was in my first year of studying IT, we had to learn all about
Binary Search Trees and Tries and Hash Tables and 2-3 Trees. Seeing as
i'm still studying that degree and don't have that much real world
experience, I was just wondering, how often are these used? The seem to
be probably a bit more efficient than using strings and regexes,
especially when you start getting into the hundreds or thousands of ids.

In the real world, we use these structures too.  However, in the real world we only build them infrequently.  In most cases we either get a generic implementation from a library or we have the data on a database which implements all these things under the hood.

I'd guess that a lot of good developers have only written an algorithm like that once or twice professionally.

Comma seperated lists of value are pretty common.  In most cases storing and moving data in some human readable form will simplify debugging and maintenence even if it is less efficient.  So unless the feature is critical to the performance of the application, people will use a very primitive data structure like a comma seperated list.  Also, strings are fairly portable between different kinds of software, so it is common to knit together components which may have been written in different languages using strings.

A funny thing happened in the IT world.  In the 60s and 70s, lots of research went into making algorithms and processes as fast and efficient as possible.  In that period, hardware was expensive and developers were cheap, and so it was best to optimize for the best use of the expensive resource.In the 90s and this decade, the pendulum has swung.  Hardware is cheap and developers are expensive.  Having a developer spend time trying to make a reasonably fast process faster is usually fool-hardy.  And having a developer implement a custom sort routine when the library has one that is almost as fast and fully tested is lunacy.  Research, in reflection of this, now has moved onto developer productivity.