[Top][All Lists]

[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: Gecko/20060506 Firefox/ (Debian-1.5.dfsg+

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:


  Message sent via/by Savannah

reply via email to

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