MEANS: Improve error messages and coding style.
[pspp] / src / language / stats / means.c
index 27a2457052243ce71b896cabd7d722570cf08b74..c1337ad949fbb63d8dd3560d759a075b935707d5 100644 (file)
@@ -31,6 +31,7 @@
 #include "libpspp/pool.h"
 
 #include "language/command.h"
+#include "language/lexer/lexer.h"
 
 #include "count-one-bits.h"
 #include "count-leading-zeros.h"
@@ -234,7 +235,7 @@ dump_cell (const struct cell *cell, const struct mtable *mt, int level)
 static void
 dump_indeces (const size_t *indexes, int n)
 {
-  for (int i = 0 ; i < n; ++i)
+  for (int i = 0; i < n; ++i)
     {
       printf ("%ld; ", indexes[i]);
     }
@@ -300,7 +301,7 @@ generate_cell (const struct means *means,
               const struct workspace *ws)
 {
   int n_vars = count_one_bits (not_wild);
-  struct cell *cell = xzalloc ((sizeof *cell));
+  struct cell *cell = XZALLOC (struct cell);
   cell->values = xcalloc (n_vars, sizeof *cell->values);
   cell->vars = xcalloc (n_vars, sizeof *cell->vars);
   cell->not_wild = not_wild;
@@ -601,7 +602,7 @@ create_table_structure (const struct mtable *mt, struct pivot_table *pt,
   int * lindexes = ws->control_idx;
   /* The inner layers are situated rightmost in the table.
      So this iteration is in reverse order.  */
-  for (int l = mt->n_layers -1; l >=0 ; --l)
+  for (int l = mt->n_layers - 1; l >= 0; --l)
     {
       const struct layer *layer = mt->layers[l];
       const struct cell_container *instances = ws->instances + l;
@@ -751,7 +752,6 @@ means_shipout_single (const struct mtable *mt, const struct means *means,
                      const struct workspace *ws)
 {
   struct pivot_table *pt = pivot_table_create (N_("Report"));
-  pt->look.omit_empty = true;
 
   struct pivot_dimension *dim_cells =
     pivot_dimension_create (pt, PIVOT_AXIS_COLUMN, N_("Statistics"));
@@ -793,7 +793,6 @@ means_shipout_multivar (const struct mtable *mt, const struct means *means,
     }
 
   struct pivot_table *pt = pivot_table_create (ds_cstr (&dss));
-  pt->look.omit_empty = true;
   ds_destroy (&dss);
 
   struct pivot_dimension *dim_cells =
@@ -870,7 +869,7 @@ control_var_missing (const struct means *means,
       const struct variable *var = layer->factor_vars[ws->control_idx[l]];
       const union value *vv = case_data (c, var);
 
-      miss = var_is_value_missing (var, vv, means->ctrl_exclude);
+      miss = (var_is_value_missing (var, vv) & means->ctrl_exclude) != 0;
       if (miss)
        break;
     }
@@ -932,7 +931,7 @@ service_cell_map (const struct means *means, const struct mtable *mt,
             {
               const struct variable *dep_var = mt->dep_vars[v];
              const union value *vv = case_data (c, dep_var);
-             if (var_is_value_missing (dep_var, vv, means->dep_exclude))
+             if (var_is_value_missing (dep_var, vv) & means->dep_exclude)
                continue;
 
               for (int stat = 0; stat < means->n_statistics; ++stat)
@@ -941,7 +940,7 @@ service_cell_map (const struct means *means, const struct mtable *mt,
                                                               NULL);
                   stat_update *su = cell_spec[means->statistics[stat]].su;
                   su (cell->stat[stat + v * means->n_statistics], weight,
-                     case_data (c, dep_var)->f);
+                     case_num (c, dep_var));
                 }
             }
         }
@@ -972,10 +971,8 @@ prepare_means (struct means *cmd)
         {
           struct workspace *ws = mt->ws + i;
          ws->root_cell = NULL;
-          ws->control_idx = xzalloc (mt->n_layers
-                                        * sizeof *ws->control_idx);
-          ws->instances = xzalloc (mt->n_layers
-                                        * sizeof *ws->instances);
+          ws->control_idx = xcalloc (mt->n_layers, sizeof *ws->control_idx);
+          ws->instances = xcalloc (mt->n_layers, sizeof *ws->instances);
           int cmb = i;
           for (int l = mt->n_layers - 1; l >= 0; --l)
             {
@@ -1051,7 +1048,7 @@ update_summaries (const struct means *means, struct mtable *mt,
          const struct variable *var = mt->dep_vars[dv];
          const union value *vv = case_data (c, var);
          /* First check if the dependent variable is missing.  */
-         if (var_is_value_missing (var, vv, means->dep_exclude))
+         if (var_is_value_missing (var, vv) & means->dep_exclude)
            summ->n_missing += weight;
          /* If the dep var is not missing, then check each
             control variable.  */
@@ -1062,7 +1059,7 @@ update_summaries (const struct means *means, struct mtable *mt,
                const struct variable *var
                  = layer->factor_vars[ws->control_idx[l]];
                const union value *vv = case_data (c, var);
-               if (var_is_value_missing (var, vv, means->ctrl_exclude))
+               if (var_is_value_missing (var, vv) & means->ctrl_exclude)
                  {
                    summ->n_missing += weight;
                    break;
@@ -1106,28 +1103,18 @@ run_means (struct means *cmd, struct casereader *input,
   post_means (cmd);
 }
 
-struct lexer;
-
 int
 cmd_means (struct lexer *lexer, struct dataset *ds)
 {
-  struct means means;
-  means.pool = pool_create ();
-
-  means.ctrl_exclude = MV_ANY;
-  means.dep_exclude = MV_ANY;
-  means.table = NULL;
-  means.n_tables = 0;
-
-  means.dict = dataset_dict (ds);
-
-  means.n_statistics = 3;
-  means.statistics = pool_calloc (means.pool, 3, sizeof *means.statistics);
-  means.statistics[0] = MEANS_MEAN;
-  means.statistics[1] = MEANS_N;
-  means.statistics[2] = MEANS_STDDEV;
-
-  if (! means_parse (lexer, &means))
+  struct means means = {
+    .pool = pool_create (),
+    .ctrl_exclude = MV_ANY,
+    .dep_exclude = MV_ANY,
+    .dict = dataset_dict (ds),
+  };
+  means_set_default_statistics (&means);
+
+  if (!means_parse (lexer, &means))
     goto error;
 
   /* Calculate some constant data for each table.  */
@@ -1139,62 +1126,58 @@ cmd_means (struct lexer *lexer, struct dataset *ds)
        mt->n_combinations *= mt->layers[l]->n_factor_vars;
     }
 
-  {
-    struct casegrouper *grouper;
-    struct casereader *group;
-    bool ok;
-
-    grouper = casegrouper_create_splits (proc_open (ds), means.dict);
-    while (casegrouper_get_next_group (grouper, &group))
-      {
-       /* Allocate the workspaces.  */
-       for (int t = 0; t < means.n_tables; ++t)
+  struct casegrouper *grouper
+    = casegrouper_create_splits (proc_open (ds), means.dict);
+  struct casereader *group;
+  while (casegrouper_get_next_group (grouper, &group))
+    {
+      /* Allocate the workspaces.  */
+      for (int t = 0; t < means.n_tables; ++t)
        {
          struct mtable *mt = means.table + t;
-         mt->summ = xzalloc (mt->n_combinations * mt->n_dep_vars
-                             * sizeof (*mt->summ));
-         mt->ws = xzalloc (mt->n_combinations * sizeof (*mt->ws));
+         mt->summ = xcalloc (mt->n_combinations * mt->n_dep_vars,
+                             sizeof *mt->summ);
+         mt->ws = xcalloc (mt->n_combinations, sizeof *mt->ws);
        }
-       run_means (&means, group, ds);
-       for (int t = 0; t < means.n_tables; ++t)
-         {
-           const struct mtable *mt = means.table + t;
+      run_means (&means, group, ds);
+      for (int t = 0; t < means.n_tables; ++t)
+        {
+          const struct mtable *mt = means.table + t;
 
-           means_case_processing_summary (mt);
-           means_shipout (mt, &means);
+          means_case_processing_summary (mt);
+          means_shipout (mt, &means);
 
-           for (int i = 0; i < mt->n_combinations; ++i)
-             {
-               struct workspace *ws = mt->ws + i;
-               if (ws->root_cell == NULL)
-                 continue;
+          for (int i = 0; i < mt->n_combinations; ++i)
+            {
+              struct workspace *ws = mt->ws + i;
+              if (ws->root_cell)
+                means_destroy_cells (&means, ws->root_cell, mt);
+            }
+        }
 
-               means_destroy_cells (&means, ws->root_cell, mt);
-             }
-         }
+      /* Destroy the workspaces.  */
+      for (int t = 0; t < means.n_tables; ++t)
+        {
+          struct mtable *mt = means.table + t;
+          free (mt->summ);
+          for (int i = 0; i < mt->n_combinations; ++i)
+            {
+              struct workspace *ws = mt->ws + i;
+              destroy_workspace (mt, ws);
+            }
+          free (mt->ws);
+        }
+    }
 
-       /* Destroy the workspaces.  */
-       for (int t = 0; t < means.n_tables; ++t)
-         {
-           struct mtable *mt = means.table + t;
-           free (mt->summ);
-           for (int i = 0; i < mt->n_combinations; ++i)
-             {
-               struct workspace *ws = mt->ws + i;
-               destroy_workspace (mt, ws);
-             }
-           free (mt->ws);
-         }
-      }
-    ok = casegrouper_destroy (grouper);
-    ok = proc_commit (ds) && ok;
-  }
+  bool ok = casegrouper_destroy (grouper);
+  ok = proc_commit (ds) && ok;
+  if (!ok)
+    goto error;
 
   pool_destroy (means.pool);
   return CMD_SUCCESS;
 
  error:
-
   pool_destroy (means.pool);
   return CMD_FAILURE;
 }