Re: Re: why is this code bad under -w option?

Color me crazy, but why do you want to spend the time calculating 2**30 when you may not even use it?!? That's just a waste of time and space.

Also, get rid of the $valid_block_type crud.

use strict;
use warnings;

my %is_valid_type = do {
    my $i = 1;
    map { $_ => 10 * $i++ } qw( KB MB GB )
};

print "Enter the pathname and filename of the file to split: ";
my $filename = <STDIN>;
chomp($filename);

my %block = map { $_ => undef } qw( size type amount );

while (1)
{
    print "Enter the block size \n (example: 400KB,10MB,1GB): ";
    chomp($block{size} = <STDIN>);

    @block{qw(amount type)} = ($block{size} =~ m/^\s*([\d.]+)\s*([a-zA
+-Z]{2})\s*$/);
    $block{type} ||= 'KB';
    $block{type} = uc $block_type;

    unless ( $is_valid_type{$block{type}} )
    {
        print "Invalid block type specified!\n\n";
        next;
    }

    $block{size} = 2 ** $is_valid_type{$block{type}};
    last;
}
[download]

This way, you defer all the calculation until you actually use it. You also get rid of excess variables and you can treat your block information as a whole instead of three related variables. (Anytime two things are related, make that relationship explicit with either an array or hash. Don't use parallel naming to tell your reader what's going on.)

Also, you weren't storing the amount of KB that the user want to work with. Did you even write a spec for this? This code looks quite spec-less ...

------
We are the carpenters and bricklayers of the Information Age.

Don't go borrowing trouble. For programmers, this means Worry only about what you need to implement.

Please remember that I'm crufty and crochety. All opinions are purely mine and all code is untested, unless otherwise specified.

Comment on Re: Re: why is this code bad under -w option? Download Code

Replies are listed 'Best First'.
Re: Re: Re: why is this code bad under -w option? by halley (Prior) on Jul 28, 2003 at 17:20 UTC
Color me crazy, but why do you want to spend the time calculating 230 when you may not even use it?!? `% perl -MO=Deparse -e 'print 230' print 1073741824; -e syntax OK` [download] (1) Calculating `2 $n`, where n is a human-scale integer, is ridiculously easy and cheap for computers to do. It's just a bit shifting operation. (2) Sub-expressions consisting of constants, such as `(230)` are computed during the compilation stage, not the runtime stage, as demonstrated above. In Perl this doesn't matter that much for runtime savings, since you must compile on every run, but why not use Perl's compiler to do the work instead of expressing yet more (possibly buggy) Perl code to be interpreted to accomplish the same calculation? (3) Yes, I said possibly buggy. In your first `map` definition, you define KB, MB and GB as consecutive multiples of ten, not consecutive powers of 1000. If a maintenance programmer would find the numeric literal or simple expression more readable than the code that derives those values, please, use the numbers! Update: Okay, you wanted 10, 20, 30 for KB, MB, GB, just to do a runtime 2$e later. I still say this doesn't save resources and it doesn't make it more readable. But I'll grant it's what you intended, and not a bug. :) -- `[ e d @ h a l l e y . c c ]`	[reply] [d/l] [select]
Re4: why is this code bad under -w option? by dragonchild (Archbishop) on Jul 28, 2003 at 17:46 UTC
In your your first map definition, you define KB, MB and GB as consecutive multiples of ten, not consecutive powers of 1000. I'm storing the powers, not the values. So, the powers are 10, 20, and 30. I then take it to the power later. Thus, it's not buggy (at least from an analysis point of view). I was suggesting another solution. In no way do I ever say that you have to do it this way or I'll come over to your house with Vino and Guido to break your kneecaps. :-) And, in fact, you're right about calculating the values ahead of time, but not for the reason you think. The best reason is if the requirements change and there's a new block size called HH that resolves to 734623 bytes. My way is a little obfuscated. Your way is more explicit. Good call! ------ We are the carpenters and bricklayers of the Information Age. Don't go borrowing trouble. For programmers, this means Worry only about what you need to implement. Please remember that I'm crufty and crochety. All opinions are purely mine and all code is untested, unless otherwise specified.	[reply]
Re: Re: Re: why is this code bad under -w option? by Grygonos (Chaplain) on Jul 28, 2003 at 17:25 UTC
ok two things, first are you refering to my code, or the "improved code". I did store the amount the user wants to work with and the code works just fine. It may not be the best code ever and that's why I brought it here...constructive criticism... secondly.. no I didn't write a spec for this code, because it had to be done quick.. sue me Some of the things you said make sense and are good practice. There are nicer ways to say some of the other things you said. If I wanted lashings about code, I would have trolled a C++ board.	[reply]
Re4: why is this code bad under -w option? by dragonchild (Archbishop) on Jul 28, 2003 at 17:52 UTC
There are nicer ways to say some of the other things you said. You're right. But, if you wanted me to say everything nicely, you need to pay me. If you want my advice for free, then take it the way I say it. I have no idea which code I'm working with. Both versions, however, use a flag to indicate that a valid type has been entered. Both versions don't store how many of that type the block should be. The former is a style mistake and the latter is a programming error. As for specs ... had you written a spec, you would have realized that you're not storing the 20 of 20KB. You would also have realized that you have no way of parsing "20 KB " or "1.3GB ". Those are serious mistakes that were commented upon by no-one else. This also ties back in to the tone I used - I found a serious design flaw. You can either complain that I didn't sugar-coat it or you can take it and make your program be the best it can be. I don't expect effusive thanks, but I can and do expect a little humility. You were the one who posted your code asking for constructive criticism, so expect constructive criticism. Don't expect me to hold your hand. (Again, that's what you get when you pay me, not when you get your advice for free.) I tend to follow the merlyn school of thought when it comes to this. :-) ------ We are the carpenters and bricklayers of the Information Age. Don't go borrowing trouble. For programmers, this means Worry only about what you need to implement. Please remember that I'm crufty and crochety. All opinions are purely mine and all code is untested, unless otherwise specified.	[reply]
Re: Re4: why is this code bad under -w option? by Grygonos (Chaplain) on Jul 28, 2003 at 18:02 UTC
`if($block_size =~ m{(^[0-9]+)}) { $block_quantity = $1; }` [download] i reposted my code with this in it and it must be below your node depth or something because its right there. My code as I posted it does accept spaces between the amount and the type. I also didn't claim this(the original) to be finished code.	[reply] [d/l]