go ahead... be a heretic | |
PerlMonks |
Re: perl do's and don'tsby Maestro_007 (Hermit) |
on Jun 28, 2001 at 21:31 UTC ( [id://92389]=note: print w/replies, xml ) | Need Help?? |
I'm going to stick to the glaring stuff. There's more, but beyond this what I can come up with goes toward the "readability vs. performance" issues, rather than empirically good vs. bad code. Also, comments would be nice.
so far, so good, -w and use strict will take you far.
I would lose most of these (see below) and put $binary in the if statement. hmm... /\D.*/. I assume you want to check for the presence of non-digits in the entry. Remember that .* checks for anything, so you're checking for "a non-digit character followed by 0 or more of anything". Besides the obvious warning about .* (see Death to Dot Star!), you just need /\D*/ Furthermore, my impression is that you want to make sure the entry contains nothing but 1s and 0s, right? With a character class, you can take care of that with [^01], and narrow this whole thing down to a single if. I'd also change the print "errmsg"; exit; to a simple die, or maybe a warn and a retry attempt: Then you can go straight to the subroutine convb2d(), and I would pass $binary to it, rather than treating the value as a global: In the subroutine, either use shift where you now have $binary, or do my $binary = shift; at the top of the subroutine. here's the complete code (tested): There's more you could do, and I'm sure you could get the whole thing down to 2 lines, but this covers the major areas. Mainly, if you have to jump through that many hoops to do input validation on such a simple problem, remember that since TMTOWTDI, there's a high probability that one of those ways is much simpler. MM UPDATE: removed $! from die at chromatic's suggestion. It was completely unnecessary, as there is no condition under which it will return usable information.
In Section
Seekers of Perl Wisdom
|
|