bug-gnu-pspp
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

PSPP-BUG: [patch #5402] RANK draft 1


From: Ben Pfaff
Subject: PSPP-BUG: [patch #5402] RANK draft 1
Date: Mon, 18 Sep 2006 12:28:08 -0700
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.4) Gecko/20060506 Firefox/1.5.0.4 (Debian-1.5.dfsg+1.5.0.4-2)

Follow-up Comment #2, patch #5402 (project pspp):

Actually, I read the code over my lunch break and I have a few
preliminary comments for you.  I didn't get a chance to actually tweak
or test or even compile any code, so substantive comments will come later.

I think the casereader, casefile, fastfile, etc. changes are fine.
You could check those in right now if you like.  Oh--but I'd prefer
that you add some test cases for cloning first.  The commented-out
case_clone in fastfilereader_clone makes me wonder...

Ditto for the dict_get_case_weight change.  In general I'm in favor of
replacing int by bool where appropriate.  It's self-documenting.

The code for RANK is much smaller than I expected.  Somehow I was
thinking it would be big.  Glad to see that I was wrong.

create_var_label gets the label wrong for SAVAGE.  Maybe you should
just create a table of rank function names and then do
     snprintf (label, lsize, "%s of %s", table[index], ranked_var_name);
Shorter code, easier to update too.

I don't like sort_criteria_single.  I'd be okay with a function to do
what this function does if it initialized a struct sort_criteria
provided by the caller.  But I don't like the idea of adding static
data here.




    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?5402>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/





reply via email to

[Prev in Thread] Current Thread [Next in Thread]