Beefy Boxes and Bandwidth Generously Provided by pair Networks
There's more than one way to do things
 
PerlMonks  

Re: ilf - input line frequency time event visualization

by graff (Chancellor)
on Jul 13, 2005 at 04:36 UTC ( #474455=note: print w/replies, xml ) Need Help??


in reply to ilf - input line frequency time event visualization

Lots of room for improvement here. First off, why do you abandon sensible line-breaks and indenting in those initial "if" blocks (just before the "while" loop)? It would be better if those were more legible. (And I'll bet it would be easier to find ways to eliminate some redundant coding there.)

Second, why not use the perl-internal time and date functions (or one of the many modules for manipulating date/time values and strings), instead of relying on an OS-dependent "date" shell command? (Yes, the "date" command could have different default output formats on different versions of unix.)

Third, that "whatchar" sub is kind of ugly and way too long. Here's how I would do it:

sub whatchar { my $diff = shift; # pass a parameter rather than checking a globa +l my $char; # return this value rather than setting a global my @zeroto1 = qw/! @ # $ % ^ & * ( )/; # characters for values <1 # (the hash mark there will generate a warning, but it's okay) if ( $diff <= 0 ) { $char = ' '; # differs from OP code } elsif ( $diff < 1 ) { $char = $zeroto1[int( $diff * 10 )]; } elsif ( $diff < 10 ) { $char = ( $diff < 9 ) ? chr( int( $diff ) + ord( '1' )) : '0'; # are you sure "234567890" is better than "123456789" ?? # ... $char = chr( int( $diff ) + ord( '0' )) seems okay to me } elsif ( $diff <= 35 ) { $char = chr( int( $diff - 10 ) + ord( 'a' )); } else { $diff = 260 if ( $diff > 260 ); $char = chr( int( $diff/10 ) - 3 + ord( 'C' )); } return $char; }
Note that in the OP code, a "$diff" value of zero had the effect of not changing the value of the global "$pchar". Was that intentional?

Also, my version is written so that the caller doesn't need to do "int($diff)" before calling "whatchar()" (not that it matters, but it just makes more sense to me that way).

Other things can be done to clean up and simplify your code -- I'm just hitting the points that leaped out at me.

update: actually one more thing does jump out at me: fix the usage message; something like this:

Usage: $0 'command line' $0 -- commmand [args ...] $0 builtin {-D|-N|-S|-W} [options ...] "builtin" functions: -D 'command line' : track "command" output -N [-n[o]] : track output of lsof (network status) ...
Be a little less verbose and exhaustive in the examples, and more "schematic" and structured in the overview of args and options, to make it clear what the command line syntax is.

Replies are listed 'Best First'.
Re^2: ilf - input line frequency time event visualization
by cider (Acolyte) on Jul 13, 2005 at 13:34 UTC
    Cool. I really appreciate the time you took to offer me this advice.

    * I had been working on this to make it a war room - http://dreamscape.org/code/ilf-war.png -- check it out! :)
    * I immediately implemented your advice regarding ditching the examples and making a more condensed usage.
    * whoops, nano -T 3 screws me again, hehe, I might as well make -T 234234234 or something so I am conditioned to allways space things out. I tediously lined everything back up. gotta love space space enter ^N enter ^N ........ ^K ^P ^P ^K ^P ^P ^K.. ugh ;)
    * I implemented your whatchar function, that's pretty cool. It was a quick hack that's almost out of diapers, and you changed a couple. I appreciate it :)
    * I grabbed a pure perl strftime and replaced the "date" execution with it's usage. I was really fond of http://dreamscape.org/code/now and it worked for me, but it wasn't exactly smart to execute it that much.

    All in all, I'm happy with it right now. Thanks for your guidance! :) checkout the http://dreamscape.org/code/ilf-war.png screenshot yo

    with very positive regards,
    -slf:) cider
    A reply falls below the community's threshold of quality. You may see it by logging in.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://474455]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others scrutinizing the Monastery: (7)
As of 2020-06-03 15:40 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    Do you really want to know if there is extraterrestrial life?



    Results (25 votes). Check out past polls.

    Notices?