No such thing as a small change PerlMonks

### Re: Can you spot the problem?

by little (Curate)
 on Mar 05, 2004 at 11:35 UTC ( #334184=note: print w/ replies, xml ) Need Help??

in reply to Can you spot the problem?

this
```if ((\$a|\$b|\$c|\$d) < 256 ) {
return 1;
}
should be
```if ((\$a < 256) && (\$b < 256) && (\$c < 256) && (\$d < 256) ) {
return 1;
}

Have a nice day
All decision is left to your taste

Comment on Re: Can you spot the problem?
Replies are listed 'Best First'.
Re: Re: Can you spot the problem?
by antirice (Priest) on Mar 05, 2004 at 16:08 UTC

Or you could just change the first if from

```if ( (\$a,\$b,\$c,\$d) = \$ip =~ m/^(\d+)\.(\d+)\.(\d+)\.(\d+)\$/ ) {

to

```if ( (\$a,\$b,\$c,\$d) = map \$_+0,\$ip =~ m/^(\d+)\.(\d+)\.(\d+)\.(\d+)\$/ )
+ {

and the code will work as expected.

antirice
The first rule of Perl club is - use Perl
The
ith rule of Perl club is - follow rule i - 1 for i > 1

Re: Re: Can you spot the problem?
by flyingmoose (Priest) on Mar 05, 2004 at 14:14 UTC
Exactly the opening post attempts to be clever, but instead it does something that doesn't even make sense... (\$a|\$b|\$c|\$d) is not any() from Quantum::Superpositions, it's an or operation on a series of values and then a conditional. Horribly bad form and it should have never been written that way in the first place.

( For the vast masses downvoting this, BTW, please explain why the OP post is exemplifying good software design... think about it... not writing maintainable working code and falling for these kind of traps is what gives Perl programmers a bad name .. and this is *not* helping. I don't mind taking the hit here, go ahead, please stand up for your right to write broken code and drag down Perl in your workplace, etc. )

... the opening post attempts to be clever, but instead it does something that doesn't even make sense...

It make sense if you understand a lower-level idiom. To be fair, I left off the thread of discussion that led up to something close to that particular snippet.

The problem, at the level of that statement, is to determine if any one of the octet numbers exceeds 255. You can compare the numbers one by one, or you can take advantage of knowing that 255 == 0xFF, and that if any one of the octet numbers is larger than that, the OR of all the numbers will have bits set above 0xFF, and will hence be greater than 255 (and conversly, a valid quad will OR to less than 256). At a raw instruction level, doing three ORs and one test/branch is faster than doing four individual test/branches if the majority of the IP addresses are going to be valid. This works well in assembler, and in C (which is a limited number of steps above assembler). If you haven't played at that level, the idiom may seem strange. And, unless performance really, really matters, it's not a technique that I'd use in production Perl. But for purposes of the puzzle, it's great, since it's so misleading.

I think that if you would like to optimize this code that much in C or assembler, you wouldn't convert the decimal strings to binary numbers at all.

That being said, I do not attack the original code, it is a nifty good idea.

instead it does something that doesn't even make sense

I disagree. Most people would expect a bitwise-or of a series of numbers. In many languages it would do exactly what the author intended.

...but we're moving hint territory :-)

Yes, and even trying for a bitwise-or (which this is not) in this particular example is bad design. The explicit if-test as little proposed is what you want.

C programmers are well known to do a x|=0x0000FFF << 7 + 3 and other wacky stuff when simpler things work. Also bad form. Yes, I'm giving the meta-response, deal with it :)

If you're talking about good software design, you shouldn't be parsing IPv4 addresses with regexen in the first place. I took the OP as a clever puzzle and nothing more. I think it's unfair to judge it on the basis of good practices.

----
: () { :|:& };:

Note: All code is untested, unless otherwise stated

For the vast masses downvoting this, BTW, please explain why the OP post is exemplifying good software design...

S'funny, but no matter how may times I re-read the OP, I can't find the reference to "good software design". From my reading, it appears to be holding the code up as an example of (at best) a gotchya, and probably bad design, but mostly, as a puzzle. Something fun to try a solve by looking rather than debugging.

Examine what is said, not who speaks.
"Efficiency is intelligent laziness." -David Dunham
"Think for yourself!" - Abigail
( For the vast masses downvoting this, BTW, please explain why the OP post is exemplifying good software design... think about it... not writing maintainable working code and falling for these kind of traps is what gives Perl programmers a bad name .. and this is *not* helping. I don't mind taking the hit here, go ahead, please stand up for your right to write broken code and drag down Perl in your workplace, etc. )
If you ever used the reason why it works for other things, it's fairly obvious. I remember seeing &sub(\$a,\$b), sub( \$a, \$b ) and sub \$a, \$b, which are all the same for the most part, but are different formats. It was really unclear when people nested paren'-less subs. But as I got more experienced, as a programmer.. as a perl developer.. things got clearer. So the mistake in the program made me wonder, ok, this doesn't work, i know WHY it shouldn't work, lemme do some examples on paper using the logical error.

But I don't think you got downvoted solely for your opinion, 'cause to some degree, you are right, it could be clearer, but then again, \$a||='bob' is too, eh?

Create A New User
Node Status?
node history
Node Type: note [id://334184]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others romping around the Monastery: (16)
As of 2016-05-04 14:04 GMT
Sections?
Information?
Find Nodes?
Leftovers?
Voting Booth?
What font do you use for programming?

Results (78 votes). Check out past polls.