Re: refine the code

Hi Lina,

On Tue, 5 Jun 2012 16:03:52 +0800
lina <lina.lastname@gmail.com> wrote:

> Hi,
> 
> I wrote one, but don't know make it mature.
> 

Overall, your program is OK, but see below for my nitpicks.

> Here is what I am going to do.
> 
> "43 43   40 1",
> "A  c #FFFFFF " /* "0" */,
> "B  c #F8F8F8 " /* "0.0385" */,
> "C  c #F2F2F2 " /* "0.0769" */,
> "D  c #EBEBEB " /* "0.115" */,
> "E  c #E5E5E5 " /* "0.154" */,
> "F  c #DEDEDE " /* "0.192" */,
> "G  c #D8D8D8 " /* "0.231" */,
> "H  c #D1D1D1 " /* "0.269" */,
> "I  c #CBCBCB " /* "0.308" */,
> "J  c #C4C4C4 " /* "0.346" */,
> "K  c #BEBEBE " /* "0.385" */,
> "L  c #B7B7B7 " /* "0.423" */,
> "M  c #B1B1B1 " /* "0.462" */,
> "N  c #AAAAAA " /* "0.5" */,
> "O  c #A3A3A3 " /* "0.538" */,
> "P  c #9D9D9D " /* "0.577" */,
> "Q  c #969696 " /* "0.615" */,
> "R  c #909090 " /* "0.654" */,
> "S  c #898989 " /* "0.692" */,
> "T  c #838383 " /* "0.731" */,
> "U  c #7C7C7C " /* "0.769" */,
> "V  c #767676 " /* "0.808" */,
> "W  c #6F6F6F " /* "0.846" */,
> "X  c #696969 " /* "0.885" */,
> "Y  c #626262 " /* "0.923" */,
> "Z  c #5C5C5C " /* "0.962" */,
> "a  c #555555 " /* "1" */,
> "b  c #4E4E4E " /* "1.04" */,
> "c  c #484848 " /* "1.08" */,
> "d  c #414141 " /* "1.12" */,
> "e  c #3B3B3B " /* "1.15" */,
> "f  c #343434 " /* "1.19" */,
> "g  c #2E2E2E " /* "1.23" */,
> "h  c #272727 " /* "1.27" */,
> "i  c #212121 " /* "1.31" */,
> "j  c #1A1A1A " /* "1.35" */,
> "k  c #141414 " /* "1.38" */,
> "l  c #0D0D0D " /* "1.42" */,
> "m  c #070707 " /* "1.46" */,
> "n  c #000000 " /* "1.5" */,
> /* x-axis:  1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
>  26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 */
> /* y-axis:  1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
>  26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 */
> "hebZWbdddXeecbYRQKKLNRVZaZWYWSOIKNOPPPNONTA",
> "nlkgjmlmlgmnkhnimjfejnnnnnjiighdigbYWSPJDAT",
> 
> Translated the first "hebZWbdddXeecbYRQKKLNRVZaZWYWSOIKNOPPPNONTA" into value.
> 
> Here is the code I wrote so far (it works)
> 
> $ more translate
> 
> #!/usr/bin/env perl
> 
> use strict;
> use warnings;
> use autodie qw(open close);
> use 5.012;
> 

These thing are great to have at the beginning of the code!

> my $file = "dm_0.xpm";
> 
> open my $fh, '<', $file;

In general, I recommend against calling identifiers, simply file, because it
can mean a "file handle", a "file name" or the "file’s contents", see:

http://perl-begin.org/tutorials/bad-elements/#calling-variables-file

> 
> my %dict;
> 
> while (<$fh>){

In general, it's preferable to iterate over lines using a lexical instead of
using the "$_" default variable which can get clobbered and devastated very
easily.

> 
> 	if(/\"[A-Za-z]  c /){

There is no need for the \ before the " in this case. 

> 		push my @array, split ' ',$_;

This is better written as:

my @array = split ' ',$_;

And you should use a more meaningful identifier than "array".

> 		foreach($array[0]){
> 			s/\"//g;
> 		};

This is better written as:

	$array[0] =~ s/"//g;

Also do you want to remove all occurences of «"» (hence the //g) or only the
first one? Should it be in the beginning of the string?

> 		foreach($array[5]){
> 			s/\"//g;
> 		}

Again:

$array[5] =~ s/"//g;

And this time, you have a magic constant:

http://perl-begin.org/tutorials/bad-elements/#magic_numbers

> 		$dict{$array[0]}=$array[5];

Since you're only matching the first and last element, you should probably do a
single regex for both, extract the first and last matches out and take that.

> 	}
> 	if(/\"([A-Za-z]+)\",/){

It is preferable to do something like:

if (my ($match) = ($line =~ ....))

instead. This way $match is lexically scoped ($1/$2/$3 and friends get reset on
the next match).

> 		push my @value, split '',$1;

Again:

my @value = split '', $match;


> 		foreach (@value){
> 			print $dict{$_}," ";
> 		}

1. You should use a lexical variable as an iterator for the foreach loop.

2. This is better written using map:

print map { "$dict{$_} " } @value;

3. You will have a trailing space. If you're not interested in it do print
join(' ', @dict{@value}).

> 		last;

I (and Perl Best Practices) prefer that every jump instruction should be
explicitly labelled, but other people disagree:

http://perl-begin.org/tutorials/bad-elements/#flow-stmts-without-labels

> 	}	
> 	
> }
> 
> The output is
> 
> 1.27 1.15 1.04 0.962 0.846 1.04 1.12 1.12 1.12 0.885 1.15 1.15 1.08
> 1.04 0.923 0.654 0.615 0.385 0.385 0.423 0.5 0.654 0.808 0.962 1 0.962
> 0.846 0.923 0.846 0.692 0.538 0.308 0.385 0.5 0.538 0.577 0.577 0.577
> 0.5 0.538 0.5 0.731 0
> 
> A further question, I have a series of those files, here this script
> only handle one.
> 
> Here is the short bash I used
> $ for f in *.xpm ; do more translate | sed s/'dm_0.xpm'/$f/g > temp ;
> chmod +x temp ; ./temp; done
> Can it be integrated into translate.pl file?
> 


Not sure I fully get to the bottom of this code, but you can write a similar
logic in Perl. See:

* http://perl-begin.org/uses/sys-admin/

* https://metacpan.org/release/IO-All

Regards,

	Shlomi Fish

-- 
-----------------------------------------------------------------
Shlomi Fish       http://www.shlomifish.org/
My Public Domain Photos - http://www.flickr.com/photos/shlomif/

Lone Starr: “Just what we need — a Druish princess.”
Barf: “Funny! She doesn’t look Druish.”
    — Spaceballs, http://www.imdb.com/title/tt0094012/

Please reply to list if it's a mailing list post - http://shlom.in/reply .

• refine the code by lina
• Re: refine the code by Shlomi Fish
• Re: refine the code by lina