יום שלישי, 14 ביוני 2011

קצר או קריא ?




יש מצבים שאנו מסתכלים על קוד והוא נראה זוועה. שמות המשתנים קצרים ולא אינדיקטיביים, אין הערות, אין חלוקה לפונקציות ובאופן כללי - בלאגן.
כשנתקלים בקוד מסוג זה, בייחוד אם זה תוך כדי חיפוש באג מציק, ובמיוחד אם חוסר הקריאות מעכבת אותנו בבואנו למצוא את התקלה, מיד לאחר מציאת התקלה יש לנו חשק עז לתקן את הקוד ולשפר לפעמים הבאות.

 אם ניקח את הקוד הבא לדוגמא:

 1 $f2 = shift or die "Missing dictionary file";
 2 
 3 open FH, "<$f2" or die $!;
 4 while(<FH>) {
 5   chomp;
 6   $h{$1} = $2 if /^(\w+) *= *(.*)$/;
 7 }
 8 close FH;
 9 
10 while(<>) {
11   s/\b\w+\b/exists $h{$&} ? $h{$&} : $&/ge;
12   print;
13 }
14 


התוכנית מקבלת קובץ מילון ובכל פעם שמגיעה בקלט מילה מהמילון, מחליפה אותה בערכה בקובץ המילון.
שכתוב פשוט על ידי חלוקה לפונקציות, הוספת הערות ושינויים קוסמטיים, הופך את הקוד להרבה יותר קריא, אך גם הרבה יותר ארוך:

 1 use strict;
 2 use warnings;
 3 
 4 my $USAGE = <<END;
 5 Usage: perl translate_long.pl <dictionary_file>
 6 END
 7 
 8 my $WORD = qr { \b\w+\b }xms;
 9 
10 my $dictionary_filename = shift or die $USAGE;
11 my %dictionary          = init_dictionary($dictionary_filename);
12 
13 while (<>) {
14   s/($WORD)/translated(\%dictionary, $1)/ge;
15   print;
16 }
17 
18 sub init_dictionary {
19   my ($filename) = @_;
20   my %dictionary;
21 
22   open my $dict_file, '<', $filename;
23 
24   while (my $line = <$dict_file>) {
25     chomp $line;
26 
27     my ($term, $value) = split / *= */, $line;
28     $dictionary{$term} = $value;
29   }
30 
31   close $dict_file;
32 
33   return %dictionary;
34 }
35 
36 sub translated {
37   my ($dictionary, $term) = @_;
38 
39   return ( exists $dictionary->{$term} ?
40                   $dictionary->{$term} : $term );
41 }
42 


עלינו מ 13 שורות ל 41 שורות. אז נכון את הקוד הראשוני היה קשה להבין, היה קשה לתקן או לאתר בו באגים ובוודאי שלא היה מה לדבר על שימוש חוזר - אבל האם באמת הרווחנו קריאות ? אני לא בטוח.

הפתרון הנכון יהיה לשפר את הקריאות בלי להאריך את הקוד, ומסתבר שזה גם אפשרי אם רק נתמקד בבעיה שלפנינו ולא בקוד התשתית שצריך להיכתב. למעשה, אפשר היה לקחת את כל הקוד שקורא את קובץ המילון ולהשתמש בו בהקשרים נוספים - מטבע הדברים, התוכנית שלנו היא לא היחידה שצריכה לקרוא קובץ מילון ולייצר ממנו טבלת hash.
הנה התוכנית בגירסתה המתוקנת, המשלבת את המודול Config::Std מ CPAN:


 1 use strict;
 2 use warnings;
 3 use Config::Std;
 4 
 5 my $USAGE = <<END;
 6 Usage: translate.pl <dictionary_file>
 7 END
 8 
 9 my $dict_filename          = shift or die $USAGE;
10 read_config $dict_filename => my $dictionary;
11 
12 # Use the default section of the file
13 my $section = $dictionary->{''};
14 
15 while (<>) {
16   s{ (\b\w+\b) }
17    { exists $section->{$1} ?
18      $section->{$1} : $1
19    }xge;
20    print;
21 }
22 


הרווחנו קריאות, לא עלינו משמעותית באורך התוכנית, ובעיקר הרווחנו שימוש במודול תשתית שנבדק ונוסה, ונותן הרבה יותר פונקציונאליות ממה שאנו תכננו לממש (מישהו אמר תמיכה בהערות בקובץ המילון ?). הקוד האמצעי היה רק אבן דרך, הוא אפשר לנו לאתר את החלקים בקוד שהיו מוכנים ל reuse. החלפה של מודולים פנימיים במודולי תשתית גנריים היא הצעד הבא והיעיל לשפר את הקוד.