Comment On SingleQautedString(), RemoveMeFunction(), Human-Friendly Dates, and More

"SpTCRUtl_showNumTelHL7" is my favorite password!!
How did he know that? Big Brother is watching me...

"While looking through some old code, I found this snippet that creats a 'human friendly' date to display to the user," Adam Smith writes, "perhaps this is more efficient than date('d/m/Y')."

// Get today's date, and also format it in a more human-friendly way
$date = date("Y-m-d");
$nicedate = preg_replace("/(....).(..).(..)/","$3/$2/$1",$date);

I'm going to wear my Ghostbusters gb_custome for Halloween.

Adam's date code was so efficient Alex had to include it twice!

Space Time Continuum:

"While looking through some old code, I found this snippet …

Please post this one again as penultimate comment.

"While looking through some old code, I found this snippet that creats a 'human friendly' date to display to the user," Adam Smith writes, "perhaps this is more efficient than date('d/m/Y')."
// Get today's date, and also format it in a more human-friendly way
$date = date("Y-m-d");
$nicedate = preg_replace("/(....).(..).(..)/","$3/$2/$1",$date);

The one by Andrew seems awfully familiar...

"While looking through some old code, I found this snippet that creats a 'human friendly' date to display to the user," Adam Smith writes, "perhaps this is more efficient than date('d/m/Y')."

// Get today's date, and also format it in a more human-friendly way
$date = date("Y-m-d");
$nicedate = preg_replace("/(....).(..).(..)/","$3/$2/$1",$date);

Stev:

"While looking through some old code, I found this snippet that creats a 'human friendly' date to display to the user," Adam Smith writes, "perhaps this is more efficient than date('d/m/Y')."

// Get today's date, and also format it in a more human-friendly way
$date = date("Y-m-d");
$nicedate = preg_replace("/(....).(..).(..)/","$3/$2/$1",$date);

"While looking through some old code, I found this snippet that creats a 'human friendly' date to display to the user," Adam Smith writes, "perhaps this is more efficient than date('d/m/Y')."
// Get today's date, and also format it in a more human-friendly way
$date = date("Y-m-d");
$nicedate = preg_replace("/(....).(..).(..)/","$3/$2/$1",$date);

A sinlge if could probably do the job, but surely you'd have to write some pretty inglorious code to achieve that.

As it stands, the code is readable and efficient, so I dont see what you'd gain by refactoring it?

eVil:

A sinlge if could probably do the job, but surely you'd have to write some pretty inglorious code to achieve that.

As it stands, the code is readable and efficient, so I dont see what you'd gain by refactoring it?

Thanks. I was starting to doubt my sanity on that one because I could not imagine the ugly if statement required to validate that in the same way.

eVil:

A sinlge if could probably do the job, but surely you'd have to write some pretty inglorious code to achieve that.

As it stands, the code is readable and efficient, so I dont see what you'd gain by refactoring it?

Although, there should probably be a check on the 2nd and third repetitions, to check whether the previous instances of that loop actually find something (currently, if the first loop finds what its looking for, it breaks the loop nicely, then somewhat stupidly enters an almost identical loop to look for something it has already found)... those checks would actually add another 2 if statements.

eVil:

A sinlge if could probably do the job, but surely you'd have to write some pretty inglorious code to achieve that.

As it stands, the code is readable and efficient, so I dont see what you'd gain by refactoring it?

Efficient? Using a loop through a contiguous range of values to check whether a value is in that range?

gobes:

Stev:

"While looking through some old code, I found this snippet that creats a 'human friendly' date to display to the user," Adam Smith writes, "perhaps this is more efficient than date('d/m/Y')."

// Get today's date, and also format it in a more human-friendly way
$date = date("Y-m-d");
$nicedate = preg_replace("/(....).(..).(..)/","$3/$2/$1",$date);

"While looking through some old code, I found this snippet that creats a 'human friendly' date to display to the user," Adam Smith writes, "perhaps this is more efficient than date('d/m/Y')."
// Get today's date, and also format it in a more human-friendly way
$date = date("Y-m-d");
$nicedate = preg_replace("/(....).(..).(..)/","$3/$2/$1",$date);

Perhaps this is more efficient than just posting it once.

eVil:

eVil:

A sinlge if could probably do the job, but surely you'd have to write some pretty inglorious code to achieve that.

As it stands, the code is readable and efficient, so I dont see what you'd gain by refactoring it?

Although, there should probably be a check on the 2nd and third repetitions, to check whether the previous instances of that loop actually find something (currently, if the first loop finds what its looking for, it breaks the loop nicely, then somewhat stupidly enters an almost identical loop to look for something it has already found)... those checks would actually add another 2 if statements.

You are joking, right? Adding Boolean algebra is inglorious?

Well, we're talking about VBScript, so it's inglorious by definition, I'll give you that.

pjt33:

eVil:

A sinlge if could probably do the job, but surely you'd have to write some pretty inglorious code to achieve that.

As it stands, the code is readable and efficient, so I dont see what you'd gain by refactoring it?

Efficient? Using a loop through a contiguous range of values to check whether a value is in that range?

Not knowing exactly which language that was, I just assumed that cint() was some kind of array of values being indexed into... is this not the case?

eVil:

A sinlge if could probably do the job, but surely you'd have to write some pretty inglorious code to achieve that.

As it stands, the code is readable and efficient, so I dont see what you'd gain by refactoring it?

Not sure about my VB syntax, but why not this :

bolMatch=(cint(val) >= 3000 and cint(val) <= 3299) or (cint(val) >= 3351 and cint(val) <= 3441) or (cint(val) >= 5964 and cint(val) <= 5969)

or that ?

if cint(val) >= 3000 and cint(val) <= 3299 then bolMatch=true
else if cint(val) >= 3351 and cint(val) <= 3441 then bolMatch=true
else if cint(val) >= 5964 and cint(val) <= 5969 then bolMatch=true

eVil:

A sinlge if could probably do the job, but surely you'd have to write some pretty inglorious code to achieve that.

As it stands, the code is readable and efficient, so I dont see what you'd gain by refactoring it?

My main point of pain in that one, is why it has to do so many iterations? That takes time! You can cut each for away and add an extra condition (or two, depending on what the language provides) to the existing IF statement. You can actually combine all three such IF statements into one, but that would indeed cost in maintainability.

Additionally, I've always been taught that breaking out of a for-loop is bad practice. Logically speaking, if your loop should end on a certain condition being met instead of running through all defined iterations, you'd need to use a while loop (either a while do or a do while, depending on) and not a for loop.

Also, frist!

Seeing now that that isn't some kind of index, I now feel sick.

God almighty.

Yes, given VB's lovely syntax, it _could_ be. However, C${TYPE} functions are used for conversion, so this probably means:
if ((int)val == (int)i);

Even though I actually often work with VBScript, I missed that too... Brain trying to escape the horror, I guess ;).

Plus its fine to break out of a for loop, so long as its obvious why you're breaking, and the loop isn't horrendously complicated.

People just *think* that its bad practice, because it similar to using 'goto's in a way... however it is a very specific form of goto.... eg one that doesn't cause a spaghetti-code problem.

A human friendly duplication of WTFs?

"I found this code at the day job (a large bank)," notes Dave, "maybe it's just me, but I would think a single 'if' statement could have done the job."

You don't even need an 'if' statement...

bolMatch = 

(Enumerable.Range(3000, 300).Where(Function(x) x = val).Count() > 0 Or _

 Enumerable.Range(3351, 91).Where(Function(x) x = val).Count() > 0 Or _

 Enumerable.Range(5964, 6).Where(Function(x) x = val).Count() > 0) 

This one was pretty awesome:
// ATTENTION: IF SOMEONE READS BLOCKS OF CODE ABOVE AND BELOW
// THIS LINE - PLEASE DONT TRY TO UNDERSTAND IT - I DON'T

There are places I may need to borrow that comment.

I'm a bit disappointed, though, because I thought Adam Smith's human-friendly date formatter WTF was going to be in today's article. Maybe next week...

Best WTF post I've seen in here in some time.

"A lot of students only go to school three quarters a year," Bob writes, "so maybe the author thought a business might take a summer off. Or try to squeeze another one in."

const QUARTERS_IN_A_YEAR = 4;

... several codefiles later...

private String[] getQuarters()
{
String[] quarters = new String[QUARTERS_IN_A_YEAR];
for (int k = 0; k < quarters.length; k++) {
quarters[k] = new Integer(k+1).toString();
}
return quarters;
}

Am I missing something? This seems to me to return a four-element array of strings, getQuarters(0) is "1", (1) is "2", (2) is "3", and (3) is "4"; it may be confusing and ugly, but still should work & not leave out any quarters ... Mostly just an illustration of why not to be rigid about your indexing standards.

I'd compile it and verify, but I have a long-standing policy of not copying any code from this site for any purpose.

This is due to the “no magic numbers” requirement being enforced to idiotic levels. Ludicrous levels would require the additional constants LOOP_START_ZERO = 0 and NEXT_INTEGER = 1, and prohibit the use of “++” and “--” operators, because 1 is also a magic number.

eVil:

Plus its fine to break out of a for loop, so long as its obvious why you're breaking, and the loop isn't horrendously complicated.

People just *think* that its bad practice, because it similar to using 'goto's in a way... however it is a very specific form of goto.... eg one that doesn't cause a spaghetti-code problem.

Just like "goto", the "break" keyword does not cause spaghetti-code problems. The problem is that people use it in spaghetti logic which causes spaghetti-code problems.

Consider Windows SDK C++ examples for SSL negotiation, which involves repeatedly invoking the same function (InitializeSecurityContext() on the client and AcceptSecurityContext() on the server) at different steps of the negotiation process. The thing is, each set requires a different combinations of parameters and have different effects, different error codes, etc.

For some reason, the developer who wrote the example noticed the process involves almost exclusively calls to the same function and thought it was a good idea to make one big loop out of this. The mess is full of "break" and "continue" statements. All structured programming. All-star spaghetti.

"A lot of students only go to school three quarters a year," Bob writes, "so maybe the author thought a business might take a summer off. Or try to squeeze another one in."

const QUARTERS_IN_A_YEAR = 4;

... several codefiles later...

private String[] getQuarters()
{
String[] quarters = new String[QUARTERS_IN_A_YEAR];
for (int k = 0; k < quarters.length; k++) {
quarters[k] = new Integer(k+1).toString();
}
return quarters;
}

The REAL WTF is that there are always 4 quarters in a year. There could be 3 trimesters, though.

I've got one colleague who tends to write things like #define N_HR 24. I've got another who tends to sprinkle magic numbers like 86400 (seconds per day) all over the place. Guess what I prefer maintaining...

Why not just

const string[] Quarters = {"1","2","3","4"}

The coder already knows that the number of quarters in a year is constant, so it's surprising he doesn't know that the actual quarters themselves also don't change!

SCSimmons:

Am I missing something? This seems to me to return a four-element array of strings, getQuarters(0) is "1", (1) is "2", (2) is "3", and (3) is "4"; it may be confusing and ugly, but still should work & not leave out any quarters ...

Think of why this function exists. Someone probably thought that:

    String[] quarters = getQuarters();

    for ( ... ) {

    }

was [i]a possible customization point[i] not provided by duplication of statements such as:

    String[] quarters = new String[]{"1", "2", "3", "4"};

    for ( ... ) {

    }

PleegWat:

I've got one colleague who tends to write things like #define N_HR 24. I've got another who tends to sprinkle magic numbers like 86400 (seconds per day) all over the place. Guess what I prefer maintaining...

I've maintained code where they put everything in #defines so as to avoid magic numbers... except for a few numbers. Go debug then! They've also added typos in the defined names, or have given it names harder to understand than N_HR. I've learned alot about magic numbers from that, namely that setting the defines should be done with care and thought.

I myself, when writing code, only make defines when the number is indeed fixed and appears in several parts of the code. Otherwise, if it's just in one function, I'll go with what your 2nd colleague does, but add a comment, saying what the number means.

frits:

"I found this code at the day job (a large bank)," notes Dave, "maybe it's just me, but I would think a single 'if' statement could have done the job."

You don't even need an 'if' statement...

bolMatch = 

(Enumerable.Range(3000, 300).Where(Function(x) x = val).Count() > 0 Or _

 Enumerable.Range(3351, 91).Where(Function(x) x = val).Count() > 0 Or _

 Enumerable.Range(5964, 6).Where(Function(x) x = val).Count() > 0) 

fixed?

if cint(val)=cint(3000) then
bolMatch=true
end if
if cint(val)=cint(3001) then
bolMatch=true
end if
if cint(val)=cint(3002) then
bolMatch=true
end if
if cint(val)=cint(3003) then
bolMatch=true
end if
if cint(val)=cint(3004) then
bolMatch=true
end if
if cint(val)=cint(3005) then
bolMatch=true
end if
if cint(val)=cint(3006) then
bolMatch=true
end if
...

The problem SingleQautedString has not found by American colleagues? What if String already contains single Quat? It needs exkaped.

Also name too long. For double quating just call Dquat and of courst this function Squat. I expert write Squat code all day.

Ok, got a new one for you:

"While looking through some old code, I found this snippet that creats a 'human friendly' date to display to the user," Adam Smith writes, "perhaps this is more efficient than date('d/m/Y')."

// Get today's date, and also format it in a more human-friendly way
$date = date("Y-m-d");
$nicedate = preg_replace("/(....).(..).(..)/","$3/$2/$1",$date);

1000 monkeys:

...


fixed?

if cint(val)=cint(3000) then
bolMatch=true
end if
if cint(val)=cint(3001) then
bolMatch=true
end if
if cint(val)=cint(3002) then
bolMatch=true
end if
if cint(val)=cint(3003) then
bolMatch=true
end if
if cint(val)=cint(3004) then
bolMatch=true
end if
if cint(val)=cint(3005) then
bolMatch=true
end if
if cint(val)=cint(3006) then
bolMatch=true
end if
...

mm..surely we would need control over the specific numbers...

how about:

 

int[] validNumbers = new int[] {3000,3001,3001,3002....}



for (int i=0; i<validNumbers.Length; i++)

    if (validNumbers[i]==i)

    {

      bolMatch=true;

      break;

    }  

create procedure dbo.SpTCRUtl_showNumTelHL7

    @numTelIn  varchar(50), 

    @numTelOut varchar(50) output

as



  set @numTelOut = @numTelIn

C'mon, you can find stuff like this in abundance any time you look at machine generated code. It's an empty template that in this instance effectively does nothing. But what the complainer didn't show you is that there's two thousand others like this, one for each data element. Many of them contain logic to manipulate the data; let's say date formatting for example. If one particular element doesn't need any formatting, you just pass it back out as it came in. Not a WTF.

if (GlobalConfig["UseSpellingError"])
{
pKey = pKey.replace("gb_custom", "gb_costum");
}

V1 had a spelling error, V1.1 fixed that but needed to keep compatibility with the widely deployed V1?

Not only are there always four quarters in a year, there are also always four quarters in a dollar, four quarters in an apple... I think the brighter ones among us can see where I'm going with this...

const QUARTERS = 4;

Done!

For the QUARTERS_IN_A_YEAR, I worked at a company that decided to switch from starting the fiscal year in October to starting it in January. That meant some contortions, making one year have 3 quarters and the next one 5. It was a one time thing but the financial software had to deal with it.

Rajeesh!:

The problem SingleQautedString has not found by American colleagues? What if String already contains single Quat? It needs exkaped.

Also name too long. For double quating just call Dquat and of courst this function Squat. I expert write Squat code all day.

I see what you did there.

eVil:

A sinlge if could probably do the job, but surely you'd have to write some pretty inglorious code to achieve that.

As it stands, the code is readable and efficient, so I dont see what you'd gain by refactoring it?

Agreed. The only sane refactor I see is extracting the repeated loop to another routine.

pjt33:

eVil:

A sinlge if could probably do the job, but surely you'd have to write some pretty inglorious code to achieve that.

As it stands, the code is readable and efficient, so I dont see what you'd gain by refactoring it?

Efficient? Using a loop through a contiguous range of values to check whether a value is in that range?

Obviously the code is only supposed to check the specified ranges, not all numbers in between.

x:

eVil:

A sinlge if could probably do the job, but surely you'd have to write some pretty inglorious code to achieve that.

As it stands, the code is readable and efficient, so I dont see what you'd gain by refactoring it?

Not sure about my VB syntax, but why not this :

bolMatch=(cint(val) >= 3000 and cint(val) <= 3299) or (cint(val) >= 3351 and cint(val) <= 3441) or (cint(val) >= 5964 and cint(val) <= 5969)

or that ?

if cint(val) >= 3000 and cint(val) <= 3299 then bolMatch=true
else if cint(val) >= 3351 and cint(val) <= 3441 then bolMatch=true
else if cint(val) >= 5964 and cint(val) <= 5969 then bolMatch=true

Sorry, but you're looping through 3000 elements wastefully. Either leave it alone or refactor out the inner loop, don't loop across the whole space and reject the ones you don't want!

"While looking through some old code, I found this snippet that creats a 'human friendly' date to display to the user," Adam Smith writes, "perhaps this is more efficient than date('d/m/Y')."

// Get today's date, and also format it in a more human-friendly way
$date = date("Y-m-d");
$nicedate = preg_replace("/(....).(..).(..)/","$3/$2/$1",$date);

TRWTF is day-first dates.

When I look into snipped Adam Smith sent, all I see is

(.) (.)

ac:

All-star spaghetti.

Some people write old-school FORTRAN in any language. Some people should be taken out back and put out of our misery. There is an overlap.

// ATTENTION: IF SOMEONE READS BLOCKS OF COMMENTS ABOVE AND BELOW
// THIS COMMENT - PLEASE DONT TRY TO UNDERSTAND IT - I DON'T

WC:

eVil:

A sinlge if could probably do the job, but surely you'd have to write some pretty inglorious code to achieve that.

As it stands, the code is readable and efficient, so I dont see what you'd gain by refactoring it?

Thanks. I was starting to doubt my sanity on that one because I could not imagine the ugly if statement required to validate that in the same way.

As always the RWTF are the comments.

integer ival = cint(val)
bolMatch = (ival >= 3000 and ival <= 3299) or
(ival >= 3351 and ival <= 3441) or
(ival >= 5964 and ival <= 5969)

In the worst case the original does:
394 loop (compare+increment) + 394 comparisons + 2*394 int conversions.
In the best case it does:
96 loop (compare+increment) + 96 comparisons + 2*96 int conversions. (if it finds the value in the first loop, the other loops are executed).

My implementation has in the best case 2 comparisons + 1 conversion in the best case (if that unrecognized language has short cut evaluation) and at most 6 comparisons and 1 conversion.

Bob:

"A lot of students only go to school three quarters a year,"

That's because their ******* retarded.

At least they didn't use month-first dates. Those are the worst kind.