Today's Magic Numbers post is a Perl

gleemonk

I'm doing a code-review for code that up to now lived in SVN. There is a mix of Perl and PHP and it has been running for more than a decade. This example is in Perl, lines 2763 to 2801:

if (($TN_1 >=29) && ($s_n_1 == '09')) {
	$s_n_1 = '06';
}
if (($TN_1 >=29) && ($s_n_1 == '07')) {
	$s_n_1 = '06';
}
if (($TN_1 >=29) && ($s_n_1 == 15)) {
	$s_n_1 = 10;
}
if (($TN_1 >=29) && ($s_n_1 == 12)) {
	$s_n_1 = 10;
}
if (($TN_1 >=27) && ($s_n_1 == '07')) {
	$s_n_1 = '06';
}
if (($TN_1 >=27) && ($s_n_1 == 12)) {
	$s_n_1 = 10;
}
if (($TN_1 <=23) && ($s_n_1 == 10)) {
	$s_n_1 = 12;
}
if (($TN_1 <=23) && ($s_n_1 == '06')) {
	$s_n_1 = '07';
}
if (($TN_1 <=23) && ($s_n_1 == 11)) {
	$s_n_1 = '07';
}
if (($TN_1 <=23) && ($s_n_1 == 10)) {
	$s_n_1 = 12;
}
if (($TN_1 <=22) && ($s_n_1 == '09')) {
	$s_n_1 = '07';
}
if (($TN_1 <=22) && ($s_n_1 == 15)) {
	$s_n_1 = 12;
}
if (($TN_1 <=23) && ($s_n_1 == 13)) {
	$s_n_1 = '07';
}

The lines above and below that block look pretty much the same except that they manipulate $s_d_1 and $s_d_2, respectively.

The consensus is that we better not touch that file

Carnage

@gleemonk said in Today's Magic Numbers post is a Perl:

The consensus is that we better not touch that file

If I'd come across that stuff in code I'd have to change, I'd rewrite it completely.
If it wouldn't be needing changes, I'd leave it alone.

Tsaukpaetra

Has the Pearl turned black yet?

HardwareGeek

@levicki said in Today's Magic Numbers post is a Perl:

@gleemonk said in Today's Magic Numbers post is a Perl:

The consensus is that we better not touch that file

Let @Tsaukpaetra touch it... with a napalm soaked cloth, and then with a gas burner.

That seems more like @Polygeekery's line of ~~work~~hobby, at least the napalm and gas part. Maybe after @Tsaukpaetra is done touching it.

gleemonk

@Tsaukpaetra when that script is sunk it won't ever be resurrected.

HardwareGeek

@levicki All the bits of that script appear to be naughty.

Tsaukpaetra

You know, I don't think I received a single mention for these last several posts...

uschwarz 0

Oooooh, I like how they apparently have a linter that warns about octal constants. (But frankly, a proper linter can just fail on seeing "perl" in the first line or your program, IYAM.)

topspin

@Tsaukpaetra said in Today's Magic Numbers post is a Perl:

You know, I don't think I received a single mention for these last several posts...

Is this a bug report or are you being sarcastic? (In which case, welcome to our world)

Tsaukpaetra

@topspin said in Today's Magic Numbers post is a Perl:

@Tsaukpaetra said in Today's Magic Numbers post is a Perl:

You know, I don't think I received a single mention for these last several posts...

Is this a bug report or are you being sarcastic? (In which case, welcome to our world)

Somewhat. Looking through the notifications log proper, it appears I would have seen two notifications, but none that appeared in the dropdown.

gleemonk

@uschwarz-0 said in Today's Magic Numbers post is a Perl:

Oooooh, I like how they apparently have a linter that warns about octal constants.

The values in these variables are printed at the end and must be zero-padded to two digits in the output. I must assume that the perpetrator found out the hard way that you can print 10 but that print 01 will drop the 0. At that point the special cases 08 and 09 were probably not that interesting.

(But frankly, a proper linter can just fail on seeing "perl" in the first line or your program, IYAM.)

E_LINTING_FUTILE

anonymous234

@Carnage said in Today's Magic Numbers post is a Perl:

@gleemonk said in Today's Magic Numbers post is a Perl:

The consensus is that we better not touch that file

If I'd come across that stuff in code I'd have to change, I'd rewrite it completely.
If it wouldn't be needing changes, I'd leave it alone.

Wrap the code into a function. Copy paste into a new function. Modify/rewrite new function as needed. New function calls old function at the end to compare results. If the result is different, log an error message and use the old result.

wft

@gleemonk I thought it was PHP. Because in Perl, this comparison:

$s_n_1 == '09'

is equivalent to

$s_n_1 == 9

in which case I suspect someone was just hammering poor code until it gave them a result they wanted. It all looks like hand-wired complicated state machine otherwise.

gleemonk

@wft Yep this code is confusing. What I understood: These variables are catenated with other strings at the end. If the zero-padding isn't there the result is wrong. So instead of consistently using strings the author used strings where otherwise the zero would be lost. I don't think they realized what Perl is doing when for example $s_d_1 . "_" . $s_n_1 drops the zero from the values they so clearly did write with a zero. All they knew is that adding quotes fixed it.

Steve_The_Cynic

@gleemonk said in Today's Magic Numbers post is a Perl:

@wft Yep this code is confusing. What I understood: These variables are catenated with other strings at the end. If the zero-padding isn't there the result is wrong. So instead of consistently using strings the author used strings where otherwise the zero would be lost. I don't think they realized what Perl is doing when for example $s_d_1 . "_" . $s_n_1 drops the zero from the values they so clearly did write with a zero. All they knew is that adding quotes fixed it.

Dunno. Maybe it was someone who went directly from BASIC to Perl, pausing along the way to wonder (but not investigate) why all the variables are strings and why all the "$" signs are at the beginning rather than the end.

wft

@gleemonk if they mean to use those “numbers” as strings, really, they need to use string comparison (eq) and concatenation everywhere. For Perl, "012_100_124" + "18" gives 30. It will just take the part of the string, starting at the beginning, which looks like a number, and throw away all the rest — that how it coerces a scalar to its numeric form.

dkf

@wft Assuming it doesn't interpret 012 as octal and convert it to 10...

wft

@dkf Nope. (I’ve checked.) If it interpreted numbers according to how numeric literals can look in the code, it would also treat _ as digit separator, and 100_000 would give 100000. It gives 100.

gleemonk

@wft said in Today's Magic Numbers post is a Perl:

@gleemonk if they mean to use those “numbers” as strings, really, they need to use string comparison (eq)

Oh that's a I wasn't even aware of! I was just irritated by the inconsistent use of quotes which meant that it would keep comparing strings and numbers. The values are more like identifiers than numbers. Though some do have an ordering.