[Koha-devel] 1.2.3RC11: Minor comments

[rbrown64 at csc.com.au]

Minor comments from a quick look at the code.

A number of the files have a large number of trailing blanks - as though
they've been editted with a line editor that leaves files as fixed width.
Please consider trimming trailing blanks for the release - when they push
the line length over 80-bytes as they do in a number of cases they make
it harder to read.

find . -name '*.p[lm]*' -print | xargs egrep -l '    *$' /dev/null


Within modules/C4/Search.pm the code to escape apostrophes should be
doing a global replace.
  $search->{'keyword'}=~ s/'/\\'/;
should be
  $search->{'keyword'}=~ s/'/\\'/g;
For smaller search SQL the search subroutines could deduplicate key words
so that 'The cat sat on the mat' doesn't look for 'the' twice.
It would make sense to generalize the 'NZ' <=> 'New Zealand' abbreviation
expansion. I'm not sure whether the current implementation adequately
uses the restrictions on other keywords. (ie whether 'New Zealand Birds'
gets you everything with NZ in it).

The code after the call to modules/C4/Search.pm:itemcount in the following
files is quite similar.

intranet-cgi/acqui/newbasket2.pl
intranet-cgi/subjectsearch.pl
opac-cgi/subjectsearch.pl

Consider providing an alternative method in Search.pm that gives
the formatted counts for On Loan, Levin, Foxton ... as a string
with a parameter for the extra stuff in newbasket2
(Lost, Mending, In Transiit (sp?)

      if ($transit > 0){
        $stuff[5]=$stuff[5]."In Transiit";
         if ($transit >1 ){

          $stuff[5]=$stuff[5]." ($transit)";

         }

         $stuff[5].=" ";
      }

I would have been tempted to write this as
    if ($translit > 0) {
      $stuff[5] .= 'In Transiit ' . (($transit > 1) ? "($transit) " : '');
    }
or even
   $stuff[5] .= 'In Transiit ' . (($transit > 1) ? "($transit) " : '')
        if $translit > 0;
but the latter is a bit write only.

The CheckDigit Excel spreadsheet http://developer.koha.org/HLT2.xls
is inaccessable.
"Forbidden
You don't have permission to access /HLT2.xls on this server."

The best reference I've seen on check digits is
Error Detecting Decimal Digits, Neal R Wagner and Paul S Putter,
Comm. ACM Jan 1989 Vol 32 No 1 pg 106-110.

Hope this is useful.
Regards,