Re: Import excel file

[John Darrington]

On Thu, Apr 14, 2011 at 12:25:13AM -0300, Michel Boaventura wrote:
     It is now working, but not complete. I just ignore any parameters that
     syntax give to me, and all variables are 255 width strings.
     Also, non asci chars are shown as '?'. But, it is working quite well.
     
     I'm sending the two new files xls-reader.[ch] and a diff of what I've
     changed to make it work. I would like very much some advices on code 
styling
     and related
     issues.
     
The code looks good.  The only comments I have are:

     convert_xls_to_value(struct ccase *c, const struct variable *var, xlsCell 
*cell)

Can "cell" ever be NULL?   Perhaps we should check and insert SYSMIS  in that 
case.

In this function, instead of ief ... else ...if else.
t might be easier to read using switch (cell->id) 

       char *tmp = malloc(width);

You're not freeing tmp, so this is going to leak memory badly.
     
       if (cell->id == 0x27e || cell->id == 0x0BD || cell->id == 0x203)

Do we know what these magic numbers mean?  If libxls doesn't have defined 
sybmbols, then I suggest that 
we invent our own and use them instead of literal 0x??? numbers.  It'll make 
the code more readable.

         sprintf(tmp,"%.15g", cell->d);

Throughout this function it might be safer to use snprintf instead of sprintf.
     
       /* Formula */
       else if (cell->id == 0x06)
       {
         /* Numeric Formula */
         if (cell->l == 0)
            sprintf(tmp,"%.15g", cell->d);
         else
         {
           /* Boolean Formula */
           if (strcmp(cell->str,"bool"))
Shouldn't this read if (0  == strcmp (cell->str, "bool") ????
             {
               if(cell->d)
                 sprintf(tmp,"true");
               else
                 sprintf(tmp,"false");
             }
           /* Error Formula */
           else if (strcmp(cell->str,"error"))
Same here..
             sprintf(tmp,"*error*");
           /* Probably a String Formula */
           else
             sprintf(tmp,"%s",cell->str);
         }
       }
       /* String? */
       else if (cell->str != NULL)
         tmp = cell->str;
       /*Empty Cell*/
       else  
         sprintf(tmp, "%s", "");
       value_copy_str_rpad(v, width, (const uint8_t *) tmp, ' ');

This cast may cause problems with non-ascii chars, but might be unavoidable.
Does libxls provide any indication of what the encoding of cell->str  will be?  
If so, we should handle it accordingly.
     }
     

Hope this helps!


J'

signature.asc
Description: Digital signature