Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #101741 > unrolled thread

Program to check for valid names

Back to article view | Back to comp.lang.c

Contents

  Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-14 04:35 -0800
    Re: Program to check for valid names Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-01-14 15:02 +0000
      Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-14 07:29 -0800
        Re: Program to check for valid names supercat@casperkitty.com - 2017-01-14 10:17 -0800
        Re: Program to check for valid names Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-01-14 18:29 +0000
          Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-16 02:16 -0800
            Re: Program to check for valid names Richard Heathfield <rjh@cpax.org.uk> - 2017-01-16 11:40 +0000
              Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-16 03:50 -0800
                Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-16 04:21 -0800
                  Re: Program to check for valid names Richard Heathfield <rjh@cpax.org.uk> - 2017-01-16 12:56 +0000
                    Re: Program to check for valid names Richard Heathfield <rjh@cpax.org.uk> - 2017-01-16 12:59 +0000
                    Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-16 05:03 -0800
                      Re: Program to check for valid names Richard Heathfield <rjh@cpax.org.uk> - 2017-01-16 13:06 +0000
                    Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-16 06:40 -0800
                      Re: Program to check for valid names Richard Heathfield <rjh@cpax.org.uk> - 2017-01-16 15:03 +0000
                        Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-16 08:29 -0800
                          Re: Program to check for valid names Richard Heathfield <rjh@cpax.org.uk> - 2017-01-16 16:42 +0000
                            Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-16 09:03 -0800
                              Re: Program to check for valid names Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-01-16 17:17 +0000
                                Re: Program to check for valid names Richard Heathfield <rjh@cpax.org.uk> - 2017-01-16 17:20 +0000
                                Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-16 09:26 -0800
                              Re: Program to check for valid names Richard Heathfield <rjh@cpax.org.uk> - 2017-01-16 17:18 +0000
                                Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-16 09:23 -0800
                                  Re: Program to check for valid names Richard Heathfield <rjh@cpax.org.uk> - 2017-01-16 17:30 +0000
                    Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-16 06:42 -0800
                      Re: Program to check for valid names Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-01-16 16:23 +0000
                        Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-16 08:46 -0800
                        Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-16 09:33 -0800
                          Re: Program to check for valid names Richard Heathfield <rjh@cpax.org.uk> - 2017-01-16 17:37 +0000
                            Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-16 09:54 -0800
                        Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-17 03:44 -0800
                          Re: Program to check for valid names Barry Schwarz <schwarzb@dqel.com> - 2017-01-17 08:16 -0800
                            Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-18 10:21 -0800
                              Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-18 10:32 -0800
                                Re: Program to check for valid names Richard Heathfield <rjh@cpax.org.uk> - 2017-01-18 19:38 +0000
                                  Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-18 12:52 -0800
                                Re: Program to check for valid names Barry Schwarz <schwarzb@dqel.com> - 2017-01-18 18:52 -0800
                                  Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-20 02:00 -0800
                                    Re: Program to check for valid names Barry Schwarz <schwarzb@dqel.com> - 2017-01-20 12:09 -0800
                                  Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-20 02:45 -0800
                                    Re: Program to check for valid names Barry Schwarz <schwarzb@dqel.com> - 2017-01-20 12:24 -0800
                                      Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-21 09:02 -0800
                              Re: Program to check for valid names Barry Schwarz <schwarzb@dqel.com> - 2017-01-18 18:05 -0800
                      Re: Program to check for valid names Barry Schwarz <schwarzb@dqel.com> - 2017-01-16 10:23 -0800
                        Re: Program to check for valid names Alla _ <modelling.data@gmail.com> - 2017-01-16 23:03 -0800
                          Re: Program to check for valid names Barry Schwarz <schwarzb@dqel.com> - 2017-01-17 00:19 -0800
    Re: Program to check for valid names Barry Schwarz <schwarzb@dqel.com> - 2017-01-14 18:23 -0800

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

#101990

On Monday, January 16, 2017 at 8:17:09 PM UTC+3, Ben Bacarisse wrote:
> Alla _ <modelling.data@gmail.com> writes:
> 
> > On Monday, January 16, 2017 at 7:42:52 PM UTC+3, Richard Heathfield wrote:
> >> On 16/01/17 16:29, Alla _ wrote:
> >> > On Monday, January 16, 2017 at 6:03:56 PM UTC+3, Richard Heathfield wrote:
> <snip>
> >> >> char *token = skipleadingspaces(line);
> >> >>
> >> >> or whatever. Then you *know* that free(line); will work fine at the end.
> >> >>
> >> > Done. The first version above, because if I use char *token =
> >> > skipleadingspaces(line);
> >> > I'd still need to "rewind" line back to the beginning,
> >> 
> >> Why?
> >> 
> >> This is a REALLY IMPORTANT QUESTION, and it seems to me that it probably 
> >> betrays a fundamental misunderstanding on your point of a vital part of 
> >> C programming.
> >> So: WHY do you think this?
> 
> The question is was about your concern over this code:
> 
>   char *token = skipleadingspaces(line);
> 
> after which a call of free(line) is safe and correct, but your reply is
> about this code
> 
> > line = skip_spaces(line);
> 
> after which it is not safe to free(line).
Yes, I am sorry - I have addressed my mistake in previous reply. 
I completely understand it now. 

[toc] | [prev] | [next] | [standalone]

#101986

On 16/01/17 17:03, Alla _ wrote:
> On Monday, January 16, 2017 at 7:42:52 PM UTC+3, Richard Heathfield wrote:
>> On 16/01/17 16:29, Alla _ wrote:
>>> On Monday, January 16, 2017 at 6:03:56 PM UTC+3, Richard Heathfield wrote:
>>
>> <snip>
>>
>>>> In any case, I'd still avoid moving this pointer along the string.
>>>> Rather, use a temp:
>>>>
>>>> char *token = line;
>>>>
>>>> or
>>>>
>>>> char *token = skipleadingspaces(line);
>>>>
>>>> or whatever. Then you *know* that free(line); will work fine at the end.
>>>>
>>> Done. The first version above, because if I use char *token = skipleadingspaces(line);
>>> I'd still need to "rewind" line back to the beginning,
>>
>> Why?
>>
>> This is a REALLY IMPORTANT QUESTION, and it seems to me that it probably
>> betrays a fundamental misunderstanding on your point of a vital part of
>> C programming.

(I wasn't sure whether my diagnosis was correct, which was why I used 
weasel words like "probably". I am even less sure now that it is true, 
but I'm keeping an open mind, just in case. We shall see...)

>> So: WHY do you think this?
>
> At the beginning of the while loop (if I don't use a temp char *token) my
> line points at the beginning of the line, read from a file. When I call skip_spaces
> function and then assign to line the return value by
> line = skip_spaces(line);
> I get the address of the first character after all beginning spaces.
> For example,
> spacespacespaceline
> after skip_spaces line will point at the address of the character "l";

Yes, but I'm suggesting that you leave line pointing at the original 
memory-allocated space, and use token instead:

token = skip_spaces(line);

so that line remains untouched.

Use token throughout. That way, line never changes, and you can 
free(line) at the end. Do you see that?

> therefore when I call free() at the end (if I have not used any other function
> to move line further), I have to rewind line back to the beginning,

Not if you never moved it in the first place, which is what I'm 
suggesting you do.

To be clear, my suggestion is this:

char *line = malloc(whatever);
if(line != NULL)
{
   while(fgets(line, len, fp) != NULL)
   {
     token = skip_spaces(line);
     do_stuff_with(token);
     token += whatever;
     token = new_value(token); etc etc

     NB line never changes
   } /* end of while loop */

   free(line);
}

Thus, you leave line ALWAYS pointing at the allocated memory, and use 
token to do your skipping along the data.

Do you see why this is easier to keep track of? Do you see that, this 
way, you don't have to remember to reset line, because you never changed 
it in the first place?

(If you do see this, then my previous SHOUTY WARNING was a 
misunderstanding on my part. I will gladly tell you what I was worried 
about if you wish, but it probably doesn't matter now.)

-- 
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within

[toc] | [prev] | [next] | [standalone]

#101989

On Monday, January 16, 2017 at 8:18:34 PM UTC+3, Richard Heathfield wrote:
> On 16/01/17 17:03, Alla _ wrote:
> > On Monday, January 16, 2017 at 7:42:52 PM UTC+3, Richard Heathfield wrote:
> >> On 16/01/17 16:29, Alla _ wrote:
> >>> On Monday, January 16, 2017 at 6:03:56 PM UTC+3, Richard Heathfield wrote:
> >>
> >> <snip>
> >>
> >>>> In any case, I'd still avoid moving this pointer along the string.
> >>>> Rather, use a temp:
> >>>>
> >>>> char *token = line;
> >>>>
> >>>> or
> >>>>
> >>>> char *token = skipleadingspaces(line);
> >>>>
> >>>> or whatever. Then you *know* that free(line); will work fine at the end.
> >>>>
> >>> Done. The first version above, because if I use char *token = skipleadingspaces(line);
> >>> I'd still need to "rewind" line back to the beginning,
> >>
> >> Why?
> >>
> >> This is a REALLY IMPORTANT QUESTION, and it seems to me that it probably
> >> betrays a fundamental misunderstanding on your point of a vital part of
> >> C programming.
> 
> (I wasn't sure whether my diagnosis was correct, which was why I used 
> weasel words like "probably". I am even less sure now that it is true, 
> but I'm keeping an open mind, just in case. We shall see...)
> 
> >> So: WHY do you think this?
> >
> > At the beginning of the while loop (if I don't use a temp char *token) my
> > line points at the beginning of the line, read from a file. When I call skip_spaces
> > function and then assign to line the return value by
> > line = skip_spaces(line);
> > I get the address of the first character after all beginning spaces.
> > For example,
> > spacespacespaceline
> > after skip_spaces line will point at the address of the character "l";
> 
> Yes, but I'm suggesting that you leave line pointing at the original 
> memory-allocated space, and use token instead:
> 
> token = skip_spaces(line);
> 
> so that line remains untouched.
Yes! My mistake. Indeed, this way I assign the result of the function
to token, and only pass the line as an argument. Yes. Sorry.
> 
> Use token throughout. That way, line never changes, and you can 
> free(line) at the end. Do you see that?
> 
> > therefore when I call free() at the end (if I have not used any other function
> > to move line further), I have to rewind line back to the beginning,
> 
> Not if you never moved it in the first place, which is what I'm 
> suggesting you do.
> 
> To be clear, my suggestion is this:
> 
> char *line = malloc(whatever);
> if(line != NULL)
> {
>    while(fgets(line, len, fp) != NULL)
>    {
>      token = skip_spaces(line);
>      do_stuff_with(token);
>      token += whatever;
>      token = new_value(token); etc etc
> 
>      NB line never changes
>    } /* end of while loop */
> 
>    free(line);
> }
> 
> Thus, you leave line ALWAYS pointing at the allocated memory, and use 
> token to do your skipping along the data.
> 
> Do you see why this is easier to keep track of? Do you see that, this 
> way, you don't have to remember to reset line, because you never changed 
> it in the first place?
> 
> (If you do see this, then my previous SHOUTY WARNING was a 
> misunderstanding on my part. I will gladly tell you what I was worried 
> about if you wish, but it probably doesn't matter now.)
> 
> -- 
> Richard Heathfield
> Email: rjh at cpax dot org dot uk
> "Usenet is a strange place" - dmr 29 July 1999
> Sig line 4 vacant - apply within

[toc] | [prev] | [next] | [standalone]

#101991

On 16/01/17 17:23, Alla _ wrote:
> On Monday, January 16, 2017 at 8:18:34 PM UTC+3, Richard Heathfield wrote:
<snip>
>>
>> token = skip_spaces(line);
>>
>> so that line remains untouched.
> Yes! My mistake. Indeed, this way I assign the result of the function
> to token, and only pass the line as an argument.

My concern, which turns out to be ill-founded, was that you somehow 
thought that skip_spaces(line) could change the value of line. It can't. 
It can only change the value of its local copy thereof, and that local 
copy disappears when the function returns.

If this /had/ been the way you were thinking, it would have betrayed a 
major misunderstanding. In C, function arguments are *ALWAYS* passed by 
value, and a C function argument can never be permanently changed solely 
by a function call. This is just as true of pointer arguments as it is 
of numeric arguments.

(The thing a pointer argument points to is another matter, obviously.)

-- 
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within

[toc] | [prev] | [next] | [standalone]

#101965

On Monday, January 16, 2017 at 3:56:44 PM UTC+3, Richard Heathfield wrote:
> On 16/01/17 12:21, Alla _ wrote:
> <snip>
<snip>
I have forgotten to attach a program. Please, ignore one tiny additional
function I have added. I need it for further work.
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>
#include <stdbool.h>
#include <errno.h>

#define LINE_SIZE 100
#define INPUT_FILE "../files/ex_1.csv"

bool process_line(FILE *input_file);
bool valid_names(const char *line);
char *skip_spaces(char *line);
char *find_first_number(char *line);

int main(int argc, char *argv[])
{
    int rc = EXIT_FAILURE;
    //Check for correct number of arguments
    if(argc > 2)
    {
        fputs("Sorry, the program processes only one file.\n", stderr);
        return rc;
    }
    //Determine a file to use
    const char *data_file = (argc == 2)? argv[1] : INPUT_FILE;
    errno = 0;
    FILE *fp = fopen(data_file, "r");
    //Couldn't open the file?
    if ( fp )
    {
        if( process_line(fp) )
            rc = EXIT_SUCCESS;
        fclose(fp);
    }
    else
        fprintf(stderr, "%s - %s\n", data_file, errno ? strerror(errno) : "Open failure!");
    return rc;
}

/** Function to find if file contains valid names **/
bool process_line (FILE *input_file)
{
    char *line = malloc(LINE_SIZE);
    char *beginning = line;
    if(!line)
    {
        printf("Error! memory not allocated.");
        return false;
    }
    size_t line_num = 0;
    
    //Parse file data, check if line contains valid names
    while( fgets(line, LINE_SIZE, input_file) != NULL)
    {
        line = skip_spaces(line);
        
        puts("****************************");
        if(valid_names(line))
        {
            line = find_first_number(line);
            printf("Success. Buffer %s\n", line);
        }
        line_num++;
    }
    line = beginning;
    if (ferror(input_file))
    {
        fprintf(stderr, "IO error after line %zu.\n", line_num);
        free(line);
        return false;
    }
    free(line);
    return true;
}

/** Function to skip spaces in the input **/
char *skip_spaces(char *line)
{
    while(isspace(*line))
        line++;
    return line;
}

/** Function to compare names to
    the required set of names 
 **/
bool valid_names(const char *line)
{
    char *names [] = { "Years", "Sales", "Cost of Goods Sold", "COGS",
        "Gross Profit", "\"Selling, General, and Admin Exp\"", "SG&A",
        "Operating Income before Depr", "Operating Income before Depreciation",
        "Depreciation and Amortization", "Operating Profit", "Interest Expense",
        "Other Gains and Losses", "Pretax Income", "Income Tax Expense", "Net Income",
        NULL};
    
    size_t i = 0;
    while(names[i])
    {
        if(!strncmp(names[i], line, strlen(names[i])))
        {
            printf("Names[%zu] = %zu\n", i, strlen(names[i]));
            return true;
        }
        i++;
    }
    return false;
}

/** Function to find first number **/
char *find_first_number(char *line)
{
    while(!isdigit(*line))
        line++;
    return line;
}

[toc] | [prev] | [next] | [standalone]

#101973

Alla _ <modelling.data@gmail.com> writes:

> On Monday, January 16, 2017 at 3:56:44 PM UTC+3, Richard Heathfield wrote:
>> On 16/01/17 12:21, Alla _ wrote:
>> <snip>
> <snip>
> I have forgotten to attach a program. Please, ignore one tiny additional
> function I have added. I need it for further work.

But the new function is wrong (it has a bug) but I don't really think
you will need it.

<snip>
> /** Function to compare names to
>     the required set of names 
>  **/
> bool valid_names(const char *line)
> {
>     char *names [] = { "Years", "Sales", "Cost of Goods Sold", "COGS",
>         "Gross Profit", "\"Selling, General, and Admin Exp\"", "SG&A",
>         "Operating Income before Depr", "Operating Income before Depreciation",
>         "Depreciation and Amortization", "Operating Profit", "Interest Expense",
>         "Other Gains and Losses", "Pretax Income", "Income Tax Expense", "Net Income",
>         NULL};
>     
>     size_t i = 0;
>     while(names[i])
>     {
>         if(!strncmp(names[i], line, strlen(names[i])))
>         {
>             printf("Names[%zu] = %zu\n", i, strlen(names[i]));
>             return true;
>         }
>         i++;
>     }
>     return false;
> }

According to your specification you don't need to search this list.
Your original plan was that the names must occur in order:

|  (1) check if row names completely correspond to the required ones,
|  no more or less of those - for this I assume I need an array
|  of required row names, and when reading each line from the 
|  file, make sure that line 1 row name equals the first name in 
|  the array, etc.

> /** Function to find first number **/
> char *find_first_number(char *line)
> {
>     while(!isdigit(*line))
>         line++;
>     return line;

When the execution gets here (as it must if the program is to work at all)
what one thing do you know with absolute certainty?  Do you see any
program with that fact?

> }

I don't think this function will help nearly as much as knowing where to
look for the first number.  The "valid_names" function could help you
with that.  Can you see how?

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#101979

On Monday, January 16, 2017 at 7:23:37 PM UTC+3, Ben Bacarisse wrote:
> Alla _ <modelling.data@gmail.com> writes:
> 
> > On Monday, January 16, 2017 at 3:56:44 PM UTC+3, Richard Heathfield wrote:
> >> On 16/01/17 12:21, Alla _ wrote:
> >> <snip>
> > <snip>
> > I have forgotten to attach a program. Please, ignore one tiny additional
> > function I have added. I need it for further work.
> 
> But the new function is wrong (it has a bug) but I don't really think
> you will need it.
> 
Ok... Let me look into it again )
> <snip>
> > /** Function to compare names to
> >     the required set of names 
> >  **/
> > bool valid_names(const char *line)
> > {
> >     char *names [] = { "Years", "Sales", "Cost of Goods Sold", "COGS",
> >         "Gross Profit", "\"Selling, General, and Admin Exp\"", "SG&A",
> >         "Operating Income before Depr", "Operating Income before Depreciation",
> >         "Depreciation and Amortization", "Operating Profit", "Interest Expense",
> >         "Other Gains and Losses", "Pretax Income", "Income Tax Expense", "Net Income",
> >         NULL};
> >     
> >     size_t i = 0;
> >     while(names[i])
> >     {
> >         if(!strncmp(names[i], line, strlen(names[i])))
> >         {
> >             printf("Names[%zu] = %zu\n", i, strlen(names[i]));
> >             return true;
> >         }
> >         i++;
> >     }
> >     return false;
> > }
> 
> According to your specification you don't need to search this list.
> Your original plan was that the names must occur in order:
> 
Oh! Of course! And I have the spec right in front of my eyes... How 
could I... Well. Thank you very much. I will re-write this piece now. 

> |  (1) check if row names completely correspond to the required ones,
> |  no more or less of those - for this I assume I need an array
> |  of required row names, and when reading each line from the 
> |  file, make sure that line 1 row name equals the first name in 
> |  the array, etc.
> 
> > /** Function to find first number **/
> > char *find_first_number(char *line)
> > {
> >     while(!isdigit(*line))
> >         line++;
> >     return line;
> 
> When the execution gets here (as it must if the program is to work at all)
> what one thing do you know with absolute certainty?  Do you see any
> program with that fact?
> 
> > }
> 
> I don't think this function will help nearly as much as knowing where to
> look for the first number.  The "valid_names" function could help you
> with that.  Can you see how?
> 
> -- 
> Ben.

[toc] | [prev] | [next] | [standalone]

#101992

On Monday, January 16, 2017 at 7:23:37 PM UTC+3, Ben Bacarisse wrote:
> Alla _ <modelling.data@gmail.com> writes:
> 
<snip>
> > bool valid_names(const char *line)
> > {
> >     char *names [] = { "Years", "Sales", "Cost of Goods Sold", "COGS",
> >         "Gross Profit", "\"Selling, General, and Admin Exp\"", "SG&A",
> >         "Operating Income before Depr", "Operating Income before Depreciation",
> >         "Depreciation and Amortization", "Operating Profit", "Interest Expense",
> >         "Other Gains and Losses", "Pretax Income", "Income Tax Expense", "Net Income",
> >         NULL};
> >     
> >     size_t i = 0;
> >     while(names[i])
> >     {
> >         if(!strncmp(names[i], line, strlen(names[i])))
> >         {
> >             printf("Names[%zu] = %zu\n", i, strlen(names[i]));
> >             return true;
> >         }
> >         i++;
> >     }
> >     return false;
> > }
> 
> According to your specification you don't need to search this list.
> Your original plan was that the names must occur in order:
> 
> |  (1) check if row names completely correspond to the required ones,
> |  no more or less of those - for this I assume I need an array
> |  of required row names, and when reading each line from the 
> |  file, make sure that line 1 row name equals the first name in 
> |  the array, etc.
<snip>
I have "corrected" it, but it looks horrible for now, and doesn't work.
I won't post questions yet, hope to find my mistakes. 

The only question: do you think the following approach (please, without yet
revealing the solution) sounds correct: I pass a line and a line number to
valid_names function, and check if the name in the array at the 
index = line number is the same as the name in the line?
The only caveat here is that for at least two lines I have two options
of valid names.

[toc] | [prev] | [next] | [standalone]

#101993

On 16/01/17 17:33, Alla _ wrote:
> On Monday, January 16, 2017 at 7:23:37 PM UTC+3, Ben Bacarisse wrote:
>> Alla _ <modelling.data@gmail.com> writes:
>>
> <snip>
>>> bool valid_names(const char *line)
>>> {
>>>     char *names [] = { "Years", "Sales", "Cost of Goods Sold", "COGS",
>>>         "Gross Profit", "\"Selling, General, and Admin Exp\"", "SG&A",
>>>         "Operating Income before Depr", "Operating Income before Depreciation",
>>>         "Depreciation and Amortization", "Operating Profit", "Interest Expense",
>>>         "Other Gains and Losses", "Pretax Income", "Income Tax Expense", "Net Income",
>>>         NULL};
>>>
>>>     size_t i = 0;
>>>     while(names[i])
>>>     {
>>>         if(!strncmp(names[i], line, strlen(names[i])))
>>>         {
>>>             printf("Names[%zu] = %zu\n", i, strlen(names[i]));
>>>             return true;
>>>         }
>>>         i++;
>>>     }
>>>     return false;
>>> }
>>
>> According to your specification you don't need to search this list.
>> Your original plan was that the names must occur in order:
>>
>> |  (1) check if row names completely correspond to the required ones,
>> |  no more or less of those - for this I assume I need an array
>> |  of required row names, and when reading each line from the
>> |  file, make sure that line 1 row name equals the first name in
>> |  the array, etc.
> <snip>
> I have "corrected" it, but it looks horrible for now, and doesn't work.
> I won't post questions yet, hope to find my mistakes.
>
> The only question: do you think the following approach (please, without yet
> revealing the solution) sounds correct: I pass a line and a line number to
> valid_names function, and check if the name in the array at the
> index = line number is the same as the name in the line?
> The only caveat here is that for at least two lines I have two options
> of valid names.

If you know for sure what the data must be, why do you need to read it 
in from the file in the first place? Why not just hold it in an array in 
your program instead? After all, you're going to do that anyway in order 
to check it.

What you are telling the user is "guess what you have to type, and I'll 
tell you if you guess wrong". What's the point?

-- 
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within

[toc] | [prev] | [next] | [standalone]

#101995

On Monday, January 16, 2017 at 8:37:13 PM UTC+3, Richard Heathfield wrote:
> On 16/01/17 17:33, Alla _ wrote:
> > On Monday, January 16, 2017 at 7:23:37 PM UTC+3, Ben Bacarisse wrote:
> >> Alla _ <modelling.data@gmail.com> writes:
> >>
> > <snip>
> >>> bool valid_names(const char *line)
> >>> {
> >>>     char *names [] = { "Years", "Sales", "Cost of Goods Sold", "COGS",
> >>>         "Gross Profit", "\"Selling, General, and Admin Exp\"", "SG&A",
> >>>         "Operating Income before Depr", "Operating Income before Depreciation",
> >>>         "Depreciation and Amortization", "Operating Profit", "Interest Expense",
> >>>         "Other Gains and Losses", "Pretax Income", "Income Tax Expense", "Net Income",
> >>>         NULL};
> >>>
> >>>     size_t i = 0;
> >>>     while(names[i])
> >>>     {
> >>>         if(!strncmp(names[i], line, strlen(names[i])))
> >>>         {
> >>>             printf("Names[%zu] = %zu\n", i, strlen(names[i]));
> >>>             return true;
> >>>         }
> >>>         i++;
> >>>     }
> >>>     return false;
> >>> }
> >>
> >> According to your specification you don't need to search this list.
> >> Your original plan was that the names must occur in order:
> >>
> >> |  (1) check if row names completely correspond to the required ones,
> >> |  no more or less of those - for this I assume I need an array
> >> |  of required row names, and when reading each line from the
> >> |  file, make sure that line 1 row name equals the first name in
> >> |  the array, etc.
> > <snip>
> > I have "corrected" it, but it looks horrible for now, and doesn't work.
> > I won't post questions yet, hope to find my mistakes.
> >
> > The only question: do you think the following approach (please, without yet
> > revealing the solution) sounds correct: I pass a line and a line number to
> > valid_names function, and check if the name in the array at the
> > index = line number is the same as the name in the line?
> > The only caveat here is that for at least two lines I have two options
> > of valid names.
> 
> If you know for sure what the data must be, why do you need to read it 
> in from the file in the first place? Why not just hold it in an array in 
> your program instead? After all, you're going to do that anyway in order 
> to check it.
> 
> What you are telling the user is "guess what you have to type, and I'll 
> tell you if you guess wrong". What's the point?
Yes, I know that the user has to provide a file, which contains a specific
number of lines, each of which should have a correct name, followed by
some unknown numbers, which will be used in computation. When the program 
receives a file, it should check of the input is correct before processing it. 

Apart from objective necessity based on the goal, it is also a great exercise in C. 

[toc] | [prev] | [next] | [standalone]

#102032

On Monday, January 16, 2017 at 7:23:37 PM UTC+3, Ben Bacarisse wrote:
> Alla _ <modelling.data@gmail.com> writes:
> 
Sorry for reposting the whole program again, not only the new 
function. I do so for your convenience. I hope I am not wrong in doing
it. 
I have changed the function valid_names - now it determines the required
input.
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>
#include <stdbool.h>
#include <errno.h>

#define LINE_SIZE 100
#define INPUT_FILE "../files/ex_1.csv"

bool process_line(FILE *input_file);
bool valid_names(const char *line, size_t line_num);
char *skip_spaces(char *line);

int main(int argc, char *argv[])
{
    int rc = EXIT_FAILURE;
    //Check for correct number of arguments
    if(argc > 2)
    {
        fputs("Sorry, the program processes only one file.\n", stderr);
        return rc;
    }
    //Determine a file to use
    const char *data_file = (argc == 2)? argv[1] : INPUT_FILE;
    errno = 0;
    FILE *fp = fopen(data_file, "r");
    //Couldn't open the file?
    if ( fp )
    {
        if( process_line(fp) )
            rc = EXIT_SUCCESS;
        fclose(fp);
    }
    else
        fprintf(stderr, "%s - %s\n", data_file, errno ? strerror(errno) : "Open failure!");
    return rc;
}

/** Function to find if file contains valid names **/
bool process_line (FILE *input_file)
{
    char *line = malloc(LINE_SIZE); //use malloc now to future realloc
    if(!line)
    {
        printf("Error! memory not allocated.");
        return false;
    }
    char *token = line;
    size_t line_num = 0;
    
    //Parse file data, check if line contains valid names
    while( fgets(token, LINE_SIZE, input_file) != NULL)
    {
        token = skip_spaces(token);
        //DEBUG fprintf
        fprintf(stderr, "****************************\n");
        
        if (line_num > 12)
        {
            fprintf(stderr, "Incorrect input: number of lines > 12.\n");
            return false;
        }
        if(valid_names(token, line_num))
        {
            //DEBUG fprintf
            fprintf(stderr, "Success. Buffer %s\n", token);
        }
        line_num++;
    }
    if (ferror(input_file))
    {
        fprintf(stderr, "IO error after line %zu.\n", line_num);
        free(line);
        return false;
    }
    free(line);
    return true;
}

/** Function to skip spaces in the input **/
char *skip_spaces(char *line)
{
    while(*line == ' ')
        line++;
    return line;
}

/** Function to compare names to
    the required set of names 
 **/
bool valid_names(const char *line, size_t line_num)
{
    const char *names_1 [] = { "Years", "Sales", "Cost of Goods Sold",
                               "Gross Profit", "\"Selling, General, and Admin Exp\"",
                               "Operating Income before Depr",
                               "Depreciation and Amortization",
                               "Operating Profit", "Interest Expense",
                               "Other Gains and Losses", "Pretax Income",
                               "Income Tax Expense", "Net Income",
                               NULL};
    const char *names_2 [] = { "Years", "Sales", "COGS", "Gross Profit",
                               "SG&A", "Operating Income before Depreciation",
                               "Depreciation and Amortization",
                               "Operating Profit", "Interest Expense",
                               "Other Gains and Losses", "Pretax Income",
                               "Income Tax Expense", "Net Income",
                               NULL};
    size_t len_1 = strlen(names_1[line_num]);
    size_t len_2 = strlen(names_2[line_num]);

    if(!strncmp(names_1[line_num], line, len_1) ||
       !strncmp(names_2[line_num], line, len_2))
    {
        //DEBUG fprintf
        fprintf(stderr, "Length of names_1[%zu] = %zu\n", line_num, len_1);
        fprintf(stderr, "Length of names_2[%zu] = %zu\n", line_num, len_2);
        return true;
    }
    return false;
}

[toc] | [prev] | [next] | [standalone]

#102053

On Tue, 17 Jan 2017 03:44:08 -0800 (PST), Alla _
<modelling.data@gmail.com> wrote:

>On Monday, January 16, 2017 at 7:23:37 PM UTC+3, Ben Bacarisse wrote:
>> Alla _ <modelling.data@gmail.com> writes:
>> 
>Sorry for reposting the whole program again, not only the new 
>function. I do so for your convenience. I hope I am not wrong in doing
>it. 
>I have changed the function valid_names - now it determines the required
>input.

<snip>

>/** Function to compare names to
>    the required set of names 
> **/
>bool valid_names(const char *line, size_t line_num)
>{
>    const char *names_1 [] = { "Years", "Sales", "Cost of Goods Sold",

Not only are the strings pointed to const, the pointers themselves are
const.  names_1[0] will never point to anything other than "Years".

>                               "Gross Profit", "\"Selling, General, and Admin Exp\"",
>                               "Operating Income before Depr",
>                               "Depreciation and Amortization",
>                               "Operating Profit", "Interest Expense",
>                               "Other Gains and Losses", "Pretax Income",
>                               "Income Tax Expense", "Net Income",
>                               NULL};

Since line_num can never be greater than 12, does that NULL still
serve a purpose?

>    const char *names_2 [] = { "Years", "Sales", "COGS", "Gross Profit",
>                               "SG&A", "Operating Income before Depreciation",
>                               "Depreciation and Amortization",
>                               "Operating Profit", "Interest Expense",
>                               "Other Gains and Losses", "Pretax Income",
>                               "Income Tax Expense", "Net Income",
>                               NULL};

Do you really need to rebuild these arrays 13 different times?

>    size_t len_1 = strlen(names_1[line_num]);
>    size_t len_2 = strlen(names_2[line_num]);
>
>    if(!strncmp(names_1[line_num], line, len_1) ||
>       !strncmp(names_2[line_num], line, len_2))

Your code assumes that each of the 13 different lines will always be
present and in order.  Is that guaranteed by the problem statement in
your course?  If so, why do you need to validate the name at all?

Did the instructors provide sample data?  Is it clean?  If handling
messy data (out of order, missing pieces, etc) is part of the
exercise, it might be better for this function to loop through the
name arrays and return the index of the matching entry.  For failure,
it could return a negative value.

>    {
>        //DEBUG fprintf
>        fprintf(stderr, "Length of names_1[%zu] = %zu\n", line_num, len_1);
>        fprintf(stderr, "Length of names_2[%zu] = %zu\n", line_num, len_2);
>        return true;
>    }
>    return false;
>}

-- 
Remove del for email

[toc] | [prev] | [next] | [standalone]

#102139

On Tuesday, January 17, 2017 at 7:16:29 PM UTC+3, Barry Schwarz wrote:
> On Tue, 17 Jan 2017 03:44:08 -0800 (PST), Alla _
> <modelling.data@gmail.com> wrote:
> 
> >On Monday, January 16, 2017 at 7:23:37 PM UTC+3, Ben Bacarisse wrote:
> >> Alla _ <modelling.data@gmail.com> writes:
> >> 
> >Sorry for reposting the whole program again, not only the new 
> >function. I do so for your convenience. I hope I am not wrong in doing
> >it. 
> >I have changed the function valid_names - now it determines the required
> >input.
> 
> <snip>
> 
> >/** Function to compare names to
> >    the required set of names 
> > **/
> >bool valid_names(const char *line, size_t line_num)
> >{
> >    const char *names_1 [] = { "Years", "Sales", "Cost of Goods Sold",
> 
> Not only are the strings pointed to const, the pointers themselves are
> const.  names_1[0] will never point to anything other than "Years".
Yes. Why is it incorrect? I don't need it to be changed. Each value of names
is used as a test value. I have used 2 arrays because I have struggled with 
line numbers passed to function when I had only I array with valid names. 
Anyway, I am now back to that one array. I don't use these two anymore.
> 
> >                               "Gross Profit", "\"Selling, General, and Admin Exp\"",
> >                               "Operating Income before Depr",
> >                               "Depreciation and Amortization",
> >                               "Operating Profit", "Interest Expense",
> >                               "Other Gains and Losses", "Pretax Income",
> >                               "Income Tax Expense", "Net Income",
> >                               NULL};
> 
> Since line_num can never be greater than 12, does that NULL still
> serve a purpose?
No, it doesn't ) I have deleted it. 
> 
> >    const char *names_2 [] = { "Years", "Sales", "COGS", "Gross Profit",
> >                               "SG&A", "Operating Income before Depreciation",
> >                               "Depreciation and Amortization",
> >                               "Operating Profit", "Interest Expense",
> >                               "Other Gains and Losses", "Pretax Income",
> >                               "Income Tax Expense", "Net Income",
> >                               NULL};
> 
> Do you really need to rebuild these arrays 13 different times?
> 
> >    size_t len_1 = strlen(names_1[line_num]);
> >    size_t len_2 = strlen(names_2[line_num]);
> >
> >    if(!strncmp(names_1[line_num], line, len_1) ||
> >       !strncmp(names_2[line_num], line, len_2))
> 
> Your code assumes that each of the 13 different lines will always be
> present and in order.  Is that guaranteed by the problem statement in
> your course?  If so, why do you need to validate the name at all?
> 
> Did the instructors provide sample data?  Is it clean?  If handling
> messy data (out of order, missing pieces, etc) is part of the
> exercise, it might be better for this function to loop through the
> name arrays and return the index of the matching entry.  For failure,
> it could return a negative value.
It's interesting. Indeed it could. I am saving this advice. Please, take 
a look at the version I have created. I will post it in a separate message
because it is an extended one (I have added the test for correct years);
maybe I have to start a new thread even. 
As to the spec: please, recall that it is my spec, it is not provided 
by the course; it is my "contraption" ) I have written the spec, and I 
provide the sample data. Yes, I would like to exclude incorrect input
data - if the file doesn't contain correct names, correct years, then 
I don't want the program to process that file. So I check the validity 
of the input (to some, limited extent).
> 
> >    {
> >        //DEBUG fprintf
> >        fprintf(stderr, "Length of names_1[%zu] = %zu\n", line_num, len_1);
> >        fprintf(stderr, "Length of names_2[%zu] = %zu\n", line_num, len_2);
> >        return true;
> >    }
> >    return false;
> >}
> 
> -- 
> Remove del for email

[toc] | [prev] | [next] | [standalone]

#102140

<snip>
Please, take a look at this piece. The main function is the same,
only struct and struct pointer declarations are added.
 
Below are inputs I used for test (sorry for reposting these
inputs again - I do so for your convenience):

INPUT_1 good one
Years,2012,2013,2014,2015,2016
Sales,1062,1252,1587,1934,2519
Cost of Goods Sold,654,814,1009,1190,1499
  Gross Profit,408,438,578,744,1020
"Selling, General, and Admin Exp",254,271,364,454,576
  Operating Income before Depr,154,167,214,290,444
Depreciation and Amortization,25,31,38,52,70
  Operating Profit,129,136,176,238,374
Interest Expense,4,3,3,1,4
Other Gains and Losses,0,7,10,0,-1
  Pretax Income,125,126,163,237,371
Income Tax Expense,55,52,65,92,141
  Net Income,70,74,98,145,230

INPUT_2 incorrect years
Years,2011,2012,2013,2014,2015
Sales,1062,1252,1587,1934,2519
COGS,654,814,1009,1190,1499
  Gross Profit,408,438,578,744,1020
SG&A,254,271,364,454,576
  Operating Income before Depr,154,167,214,290,444
Depreciation and Amortization,25,31,38,52,70
  Operating Profit,129,136,176,238,374
Interest Expense,4,3,3,1,4
Other Gains and Losses,0,7,10,0,-1
  Pretax Income,125,126,163,237,371
Income Tax Expense,55,52,65,92,141
  Net Income,70,74,98,145,230

INPUT_3 incorrect one
1  
  
-2.0
9,  9bz
2 
xyz
  7bz
3.0,2
2 ,4
1, -6.0   
Years,2011,2012,2013,2014,2015
  ,1062,1252,1587,1934,2519
   654,+814.98,1009,1190jz
  Gross Profit,&&408,438,578,$744,1020

"Selling, General, and Admin Exp",254,271,364,454,576%$@
         -0.154,167,214,290,444
Depreciation and Amortization,25,31,38,52,70
  Operating Profit,129,136,176,238,374
Interest Expense,4,3,3,1,4
Other Gains and Losses,0,7,10,0,-1
  Pretax Income,125,126,163,237,371
Income Tax Expense,55,52,65,92,141
  Net Income,70,74,98,145,230

Program:
/***
 * READS INPUT FROM A FILE LINE BY LINE;
 * DETERMINES IF INPUT HAS CORRECT NAMES;
 * DETERMINES IF THERE ARE CORRECT 5 YEARS
  OF HISTORIC DATA
 ***/
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>
#include <stdbool.h>
#include <errno.h>
#include <time.h>

#define LINE_SIZE 100
#define DATA_SIZE 5
#define INPUT_FILE "../files/ex_1.csv"

//For future usage; will be specified
//For now only years are filled
struct data {
    int years[DATA_SIZE];
    int sales[DATA_SIZE];
    int cogs[DATA_SIZE];
    int gross_profit[DATA_SIZE];
    int sga[DATA_SIZE];
    int oper_inc[DATA_SIZE];
    int dep_amor[DATA_SIZE];
    int oper_prof[DATA_SIZE];
    int int_exp[DATA_SIZE];
    int gain_loss[DATA_SIZE];
    int pretax_inc[DATA_SIZE];
    int tax_exp[DATA_SIZE];
    int net_inc[DATA_SIZE];
};

bool process_line(FILE *input_file, struct data *data);
int valid_names(char *line, size_t line_num);
char *skip_spaces(char *line);
void get_year(int *year);
bool check_input_years(char *line, int *years, int cur_year);

int main(int argc, char *argv[])
{
    int rc = EXIT_FAILURE;
    //Check for correct number of arguments
    if(argc > 2)
    {
        fputs("Sorry, the program processes only one file.\n", stderr);
        return rc;
    }
    //Determine a file to use
    const char *data_file = (argc == 2)? argv[1] : INPUT_FILE;
    errno = 0;
    FILE *fp = fopen(data_file, "r");
    
    if ( fp )
    {
        struct data income_statement;
        struct data *data = &income_statement;
        
        if( process_line(fp, data) )
            rc = EXIT_SUCCESS;
        fclose(fp);
    }
    else
        fprintf(stderr, "%s - %s\n", data_file, errno ? strerror(errno) : "Open failure!");
    return rc;
}

/** Function to find if file contains valid names **/
bool process_line (FILE *input_file, struct data *data)
{
    char *line = malloc(LINE_SIZE);
    if(!line)
    {
        printf("Error! memory not allocated.");
        return false;
    }
    char *token = line;
    size_t line_num = 0;
    
    //Parse file data, check if line contains valid names
    while( fgets(token, LINE_SIZE, input_file) != NULL)
    {
        token = skip_spaces(token);
        
        //DEBUG fprintf
        fprintf(stderr, "****************************\n");
        
        if (isspace(*token) || *token == '\0' || line_num > 12)
        {
            fprintf(stderr, "Incorrect input: nothing on the line,\nor number of lines > 12.\n");
            return false;
        }
        //Check if input row names are correct
        int token_len = 0;
        if((token_len = valid_names(token, line_num)) != -1
            && (token += token_len))
        {
            //Determine the current year
            int cur_year = 0;
            get_year(&cur_year);
            
            //5 years of data ending previous year?
            if(line_num == 0)
            {
                if(!(check_input_years(token, data->years, cur_year-1)))
                {
                    fprintf(stderr, "Incorrect years.\n");
                    return false;
                }
            }
        }
        line_num++;
        token = line;
    }
    if (ferror(input_file))
    {
        fprintf(stderr, "IO error after line %zu.\n", line_num);
        free(line);
        return false;
    }
    free(line);
    return true;
}

/** Function to skip spaces in the input **/
char *skip_spaces(char *line)
{
    while(*line == ' ')
        line++;
    return line;
}

/** Function to compare names to
    the required set of names 
 **/
int valid_names(char *line, size_t line_num)
{
    char *names [] = { "Years", "Sales", "Cost of Goods Sold", "COGS",
                       "Gross Profit",
                       "\"Selling, General, and Admin Exp\"", "SG&A",
                       "Operating Income before Depr",
                       "Operating Income before Depreciation",
                       "Depreciation and Amortization", "Operating Profit",
                       "Interest Expense",
                       "Other Gains and Losses", "Pretax Income",
                       "Income Tax Expense", "Net Income"};
    
    
    if(line_num == 3 || line_num == 4)
        line_num++;
    else if (line_num == 5)
        line_num += 2;
    else if(line_num >= 6)
        line_num += 3;
    
    int len_1 = strlen(names[line_num]);
    if( line_num >= 9 && !strncmp(names[line_num], line, len_1))
        return len_1 + 1;
    else if(line_num < 9)
    {
        int len_2 = strlen(names[line_num+1]);
        int num1 = strncmp(names[line_num], line, len_1);
        int num2 = strncmp(names[line_num+1], line, len_2);
        
        if(!num1)
            return len_1 + 1;
        else if (!num2)
            return len_2 + 1;
    }
    return -1;
}

/** Function to set a current year **/
void get_year(int *year)
{
    struct tm *current;
    time_t timenow;
    time(&timenow);
    current = localtime(&timenow);
    *year = current->tm_year+1900;
    return;
}

/** Function to determine correct years
    in the input 
 **/
bool check_input_years(char *line, int *years, int cur_year)
{
    char *not_converted = NULL;
    char *ptr = line;
    int count = 4;
    int year = strtol(ptr, &not_converted, 10);
    
    for(int i = 0; i < DATA_SIZE && not_converted > ptr
        && *not_converted != '\0'; i++, count--)
    {
        if(year != (cur_year - count))
            return false;
        
        years[i] = year;
        ptr = not_converted + 1;
        year = strtol(ptr, &not_converted, 10);
    }
    count++;
    return count == 0;
}

[toc] | [prev] | [next] | [standalone]

#102145

On 18/01/17 18:32, Alla _ wrote:
> <snip>
> Please, take a look at this piece.

Just a very quick look.

> //For future usage; will be specified
> //For now only years are filled
> struct data {
>     int years[DATA_SIZE];
>     int sales[DATA_SIZE];
>     int cogs[DATA_SIZE];
>     int gross_profit[DATA_SIZE];
>     int sga[DATA_SIZE];
>     int oper_inc[DATA_SIZE];
>     int dep_amor[DATA_SIZE];
>     int oper_prof[DATA_SIZE];
>     int int_exp[DATA_SIZE];
>     int gain_loss[DATA_SIZE];
>     int pretax_inc[DATA_SIZE];
>     int tax_exp[DATA_SIZE];
>     int net_inc[DATA_SIZE];
> };

Surely you must see that this is ridiculous?

Just define the struct to have one of everything:

struct data {
   int years;
   int sales;
   int cogs;
   ... etc
};

and then define an array of them, with DATA_SIZE elements.

-- 
Richard Heathfield
Email: rjh at cpax dot org dot uk
"Usenet is a strange place" - dmr 29 July 1999
Sig line 4 vacant - apply within

[toc] | [prev] | [next] | [standalone]

#102151

On Wednesday, January 18, 2017 at 10:38:08 PM UTC+3, Richard Heathfield wrote:
> On 18/01/17 18:32, Alla _ wrote:
> > <snip>
> > Please, take a look at this piece.
> 
> Just a very quick look.
> 
> > //For future usage; will be specified
> > //For now only years are filled
> > struct data {
> >     int years[DATA_SIZE];
> >     int sales[DATA_SIZE];
> >     int cogs[DATA_SIZE];
> >     int gross_profit[DATA_SIZE];
> >     int sga[DATA_SIZE];
> >     int oper_inc[DATA_SIZE];
> >     int dep_amor[DATA_SIZE];
> >     int oper_prof[DATA_SIZE];
> >     int int_exp[DATA_SIZE];
> >     int gain_loss[DATA_SIZE];
> >     int pretax_inc[DATA_SIZE];
> >     int tax_exp[DATA_SIZE];
> >     int net_inc[DATA_SIZE];
> > };
> 
> Surely you must see that this is ridiculous?
I do, hence the comment above it. 
> 
> Just define the struct to have one of everything:
> 
> struct data {
>    int years;
>    int sales;
>    int cogs;
>    ... etc
> };
> 
> and then define an array of them, with DATA_SIZE elements.
> 
> -- 
> Richard Heathfield
> Email: rjh at cpax dot org dot uk
> "Usenet is a strange place" - dmr 29 July 1999
> Sig line 4 vacant - apply within

[toc] | [prev] | [next] | [standalone]

#102171

On Wed, 18 Jan 2017 10:32:13 -0800 (PST), Alla _
<modelling.data@gmail.com> wrote:

<snip>

>/** Function to find if file contains valid names **/
>bool process_line (FILE *input_file, struct data *data)
>{
>    char *line = malloc(LINE_SIZE);
>    if(!line)
>    {
>        printf("Error! memory not allocated.");
>        return false;
>    }
>    char *token = line;
>    size_t line_num = 0;
>    
>    //Parse file data, check if line contains valid names
>    while( fgets(token, LINE_SIZE, input_file) != NULL)
>    {
>        token = skip_spaces(token);
>        
>        //DEBUG fprintf
>        fprintf(stderr, "****************************\n");
>        
>        if (isspace(*token) || *token == '\0' || line_num > 12)

I find it inconsistent that a line that starts with '\t' is caught
here but a line that starts with '?' doesn't get caught until you call
valid_names.  If you eliminated the first two tests, both conditions
would get caught by valid_names.

>        {
>            fprintf(stderr, "Incorrect input: nothing on the line,\nor number of lines > 12.\n");

On the other hand, your if statement tests for three "errors".  You
report only two of them.  How about 
     "Incorrect input: nothing on the line,\n"
     "                        invalid starting data in the line,\n"
     "                        or number of lines > 12.\n"

>            return false;
>        }
>        //Check if input row names are correct
>        int token_len = 0;
>        if((token_len = valid_names(token, line_num)) != -1
>            && (token += token_len))
>        {
>            //Determine the current year
>            int cur_year = 0;
>            get_year(&cur_year);

It would be much more idiomatic if get_year returned an int and you
coded 
     int cur_year = get_year();

>            //5 years of data ending previous year?
>            if(line_num == 0)
>            {
>                if(!(check_input_years(token, data->years, cur_year-1)))
>                {
>                    fprintf(stderr, "Incorrect years.\n");
>                    return false;
>                }
>            }
>        }
>        line_num++;
>        token = line;
>    }
>    if (ferror(input_file))
>    {
>        fprintf(stderr, "IO error after line %zu.\n", line_num);
>        free(line);
>        return false;
>    }
>    free(line);
>    return true;
>}

<snip skip_spaces>

>/** Function to compare names to
>    the required set of names 
> **/
>int valid_names(char *line, size_t line_num)
>{
>    char *names [] = { "Years", "Sales", "Cost of Goods Sold", "COGS",

It is still worth the typing effort to make the array and the strings
const and the array static.

>                       "Gross Profit",
>                       "\"Selling, General, and Admin Exp\"", "SG&A",
>                       "Operating Income before Depr",
>                       "Operating Income before Depreciation",
>                       "Depreciation and Amortization", "Operating Profit",
>                       "Interest Expense",
>                       "Other Gains and Losses", "Pretax Income",
>                       "Income Tax Expense", "Net Income"};
>    
>    
>    if(line_num == 3 || line_num == 4)
>        line_num++;
>    else if (line_num == 5)
>        line_num += 2;
>    else if(line_num >= 6)
>        line_num += 3;
>    
>    int len_1 = strlen(names[line_num]);
>    if( line_num >= 9 && !strncmp(names[line_num], line, len_1))
>        return len_1 + 1;
>    else if(line_num < 9)
>    {
>        int len_2 = strlen(names[line_num+1]);
>        int num1 = strncmp(names[line_num], line, len_1);
>        int num2 = strncmp(names[line_num+1], line, len_2);

This code has two overwhelming attributes: it is horrendous and it is
a maintenance nightmare.

In my opinion it is a monumental step backwards from previous
versions.

>        if(!num1)
>            return len_1 + 1;
>        else if (!num2)
>            return len_2 + 1;
>    }
>    return -1;
>}
>

<snip get_year>

>/** Function to determine correct years
>    in the input 
> **/
>bool check_input_years(char *line, int *years, int cur_year)
>{
>    char *not_converted = NULL;
>    char *ptr = line;
>    int count = 4;
>    int year = strtol(ptr, &not_converted, 10);
>    
>    for(int i = 0; i < DATA_SIZE && not_converted > ptr
>        && *not_converted != '\0'; i++, count--)
>    {
>        if(year != (cur_year - count))
>            return false;
>        
>        years[i] = year;
>        ptr = not_converted + 1;
>        year = strtol(ptr, &not_converted, 10);
>    }
>    count++;
>    return count == 0;

This is really obfuscated code.  You set count to 4, decrement it once
for each of 5 years, increment it once at the end, and then check for
0.  If you initialized count to 5 you could eliminate the increment
with the same results.

Is there some reason you didn't just count the number of years and
check for 5 at the end?

Of course it doesn't work if there are 6 years on the input line.  You
will then attempt to store a value into year[5] which does not exist.
You may want to add a check on count inside the second for statement
expression.

I am curious about how you decide which errors you will check for and
which you will ignore.  I am also curious why you are spending so much
effort on half-checking for errors and almost no effort on performing
the real task of processing the data.

>}

-- 
Remove del for email

[toc] | [prev] | [next] | [standalone]

#102214

On Thursday, January 19, 2017 at 5:52:09 AM UTC+3, Barry Schwarz wrote:
> On Wed, 18 Jan 2017 10:32:13 -0800 (PST), Alla _
> <modelling.data@gmail.com> wrote:
> 
> <snip>
> 
> >/** Function to find if file contains valid names **/
> >bool process_line (FILE *input_file, struct data *data)
> >{
> >    char *line = malloc(LINE_SIZE);
> >    if(!line)
> >    {
> >        printf("Error! memory not allocated.");
> >        return false;
> >    }
> >    char *token = line;
> >    size_t line_num = 0;
> >    
> >    //Parse file data, check if line contains valid names
> >    while( fgets(token, LINE_SIZE, input_file) != NULL)
> >    {
> >        token = skip_spaces(token);
> >        
> >        //DEBUG fprintf
> >        fprintf(stderr, "****************************\n");
> >        
> >        if (isspace(*token) || *token == '\0' || line_num > 12)
> 
> I find it inconsistent that a line that starts with '\t' is caught
> here but a line that starts with '?' doesn't get caught until you call
> valid_names.  If you eliminated the first two tests, both conditions
> would get caught by valid_names.
Yes, true. Corrected. 
> 
> >        {
> >            fprintf(stderr, "Incorrect input: nothing on the line,\nor number of lines > 12.\n");
> 
> On the other hand, your if statement tests for three "errors".  You
> report only two of them.  How about 
>      "Incorrect input: nothing on the line,\n"
>      "                        invalid starting data in the line,\n"
>      "                        or number of lines > 12.\n"
> 
> >            return false;
> >        }
> >        //Check if input row names are correct
> >        int token_len = 0;
> >        if((token_len = valid_names(token, line_num)) != -1
> >            && (token += token_len))
> >        {
> >            //Determine the current year
> >            int cur_year = 0;
> >            get_year(&cur_year);
> 
> It would be much more idiomatic if get_year returned an int and you
> coded 
>      int cur_year = get_year();
Thank you. Indeed. Done )
> 
> >            //5 years of data ending previous year?
> >            if(line_num == 0)
> >            {
> >                if(!(check_input_years(token, data->years, cur_year-1)))
> >                {
> >                    fprintf(stderr, "Incorrect years.\n");
> >                    return false;
> >                }
> >            }
> >        }
> >        line_num++;
> >        token = line;
> >    }
> >    if (ferror(input_file))
> >    {
> >        fprintf(stderr, "IO error after line %zu.\n", line_num);
> >        free(line);
> >        return false;
> >    }
> >    free(line);
> >    return true;
> >}
> 
> <snip skip_spaces>
> 
> >/** Function to compare names to
> >    the required set of names 
> > **/
> >int valid_names(char *line, size_t line_num)
> >{
> >    char *names [] = { "Years", "Sales", "Cost of Goods Sold", "COGS",
> 
> It is still worth the typing effort to make the array and the strings
> const and the array static.
I am sorry, but I don't understand this. Please, show the example. 
Do you mean const char *names[] when you say make the array
const? What does it mean to make strings const and array static?
> 
> >                       "Gross Profit",
> >                       "\"Selling, General, and Admin Exp\"", "SG&A",
> >                       "Operating Income before Depr",
> >                       "Operating Income before Depreciation",
> >                       "Depreciation and Amortization", "Operating Profit",
> >                       "Interest Expense",
> >                       "Other Gains and Losses", "Pretax Income",
> >                       "Income Tax Expense", "Net Income"};
> >    
> >    
> >    if(line_num == 3 || line_num == 4)
> >        line_num++;
> >    else if (line_num == 5)
> >        line_num += 2;
> >    else if(line_num >= 6)
> >        line_num += 3;
> >    
> >    int len_1 = strlen(names[line_num]);
> >    if( line_num >= 9 && !strncmp(names[line_num], line, len_1))
> >        return len_1 + 1;
> >    else if(line_num < 9)
> >    {
> >        int len_2 = strlen(names[line_num+1]);
> >        int num1 = strncmp(names[line_num], line, len_1);
> >        int num2 = strncmp(names[line_num+1], line, len_2);
> 
> This code has two overwhelming attributes: it is horrendous and it is
> a maintenance nightmare.
> 
> In my opinion it is a monumental step backwards from previous
> versions.
I agree that it looks horrible; this is why I have used two arrays. But that 
also didn't look good, although the rest of the code was better and more
concise. The version above is the one I could come up with after a few
other attempts. 
> 
> >        if(!num1)
> >            return len_1 + 1;
> >        else if (!num2)
> >            return len_2 + 1;
> >    }
> >    return -1;
> >}
> >
> 
> <snip get_year>
> 
> >/** Function to determine correct years
> >    in the input 
> > **/
> >bool check_input_years(char *line, int *years, int cur_year)
> >{
> >    char *not_converted = NULL;
> >    char *ptr = line;
> >    int count = 4;
> >    int year = strtol(ptr, &not_converted, 10);
> >    
> >    for(int i = 0; i < DATA_SIZE && not_converted > ptr
> >        && *not_converted != '\0'; i++, count--)
> >    {
> >        if(year != (cur_year - count))
> >            return false;
> >        
> >        years[i] = year;
> >        ptr = not_converted + 1;
> >        year = strtol(ptr, &not_converted, 10);
> >    }
> >    count++;
> >    return count == 0;
> 
> This is really obfuscated code.  You set count to 4, decrement it once
> for each of 5 years, increment it once at the end, and then check for
> 0.  If you initialized count to 5 you could eliminate the increment
> with the same results.
> 
> Is there some reason you didn't just count the number of years and
> check for 5 at the end?
> 
> Of course it doesn't work if there are 6 years on the input line.  You
> will then attempt to store a value into year[5] which does not exist.
> You may want to add a check on count inside the second for statement
> expression.
Done. I have also changed the for loop to while one. Looks better now. 
> 
> I am curious about how you decide which errors you will check for and
> which you will ignore.  I am also curious why you are spending so much
> effort on half-checking for errors and almost no effort on performing
> the real task of processing the data.
> 
Well, I have written a spec, which implies a set of conditions for an input
file; and I test from these conditions to hold. 
As to the effort, initially I have jumped into processing the data, but that 
led to unpleasant consequences and the necessity to change perfectly 
working idioms/pieces of code to make those suitable for my corrupt 
input. Therefore,  having understood my mistake, I have decided to make
sure I have the input I need before processing it. 
Also, it is a learning experience, so I do hope that all this effort if worth it
because I learn, I practice, etc. Anyway, I have already entered the processing
stage. 
> >}
> 
> -- 
> Remove del for email

[toc] | [prev] | [next] | [standalone]

#102257

On Fri, 20 Jan 2017 02:00:21 -0800 (PST), Alla _
<modelling.data@gmail.com> wrote:

>On Thursday, January 19, 2017 at 5:52:09 AM UTC+3, Barry Schwarz wrote:
>> On Wed, 18 Jan 2017 10:32:13 -0800 (PST), Alla _
>> <modelling.data@gmail.com> wrote:

<snip>

>> >/** Function to compare names to
>> >    the required set of names 
>> > **/
>> >int valid_names(char *line, size_t line_num)
>> >{
>> >    char *names [] = { "Years", "Sales", "Cost of Goods Sold", "COGS",
>> 
>> It is still worth the typing effort to make the array and the strings
>> const and the array static.
>I am sorry, but I don't understand this. Please, show the example. 
>Do you mean const char *names[] when you say make the array
>const? What does it mean to make strings const and array static?
>> 
>> >                       "Gross Profit",
>> >                       "\"Selling, General, and Admin Exp\"", "SG&A",
>> >                       "Operating Income before Depr",
>> >                       "Operating Income before Depreciation",
>> >                       "Depreciation and Amortization", "Operating Profit",
>> >                       "Interest Expense",
>> >                       "Other Gains and Losses", "Pretax Income",
>> >                       "Income Tax Expense", "Net Income"};

Take in steps.  

The array should not be recreated every time the function is called.
Therefore
     static char *names[] = {...};

The pointers should not be modified by any code.  Each always points
to the same string as initialized.  Therefore
     static char * const names[] = ...;
This reads as "names is an array on const pointers to char."

The strings pointed to by each element of the array should also never
change.  (In this case, the strings pointed to are string literals and
any attempt to change them causes undefined behavior but that doesn't
invalidate the concept.  Furthermore, the definition
     char *p = "abc";
is legal because when the standard was first being developed it would
have broken to much existing code if string literals were assigned the
const attribute.  Unless p is going to eventually be changed to point
to a modifiable string, it is recommended that the definition be coded
as
     const char *p = "abc";
This will allow the compiler to detect attempts to modify the
literal.)  Therefore
     static char const * const names[] = ...;
This reads as "names is an array of const pointers to const char".

As an exercise, you should figure out what the difference is between
those two and
    static char const * names[] = ...;
In particular, based strictly on the definition and not the
initialization, determine what is modifiable and what is not for each.

-- 
Remove del for email

[toc] | [prev] | [next] | [standalone]

#102215

On Thursday, January 19, 2017 at 5:52:09 AM UTC+3, Barry Schwarz wrote:
> On Wed, 18 Jan 2017 10:32:13 -0800 (PST), Alla _
> <modelling.data@gmail.com> wrote:
<snip>
> 
> >/** Function to compare names to
> >    the required set of names 
> > **/
> >int valid_names(char *line, size_t line_num)
> >{
> >    char *names [] = { "Years", "Sales", "Cost of Goods Sold", "COGS",
> 
> It is still worth the typing effort to make the array and the strings
> const and the array static.
> 
> >                       "Gross Profit",
> >                       "\"Selling, General, and Admin Exp\"", "SG&A",
> >                       "Operating Income before Depr",
> >                       "Operating Income before Depreciation",
> >                       "Depreciation and Amortization", "Operating Profit",
> >                       "Interest Expense",
> >                       "Other Gains and Losses", "Pretax Income",
> >                       "Income Tax Expense", "Net Income"};
> >    
> >    
> >    if(line_num == 3 || line_num == 4)
> >        line_num++;
> >    else if (line_num == 5)
> >        line_num += 2;
> >    else if(line_num >= 6)
> >        line_num += 3;
> >    
> >    int len_1 = strlen(names[line_num]);
> >    if( line_num >= 9 && !strncmp(names[line_num], line, len_1))
> >        return len_1 + 1;
> >    else if(line_num < 9)
> >    {
> >        int len_2 = strlen(names[line_num+1]);
> >        int num1 = strncmp(names[line_num], line, len_1);
> >        int num2 = strncmp(names[line_num+1], line, len_2);
> 
> This code has two overwhelming attributes: it is horrendous and it is
> a maintenance nightmare.
> 
> In my opinion it is a monumental step backwards from previous
> versions.
> 
<snip>
I have changed it back to the two-array version, but have eliminated
the second array - now it contains only values, which differ from the ones
in the first array. 
int valid_names(char *line, size_t line_num, char *name)
{
    if(isspace(*line) || *line == '\0')
        return -1;
    
    const char *names_1 [] = { "Years", "Sales", "Cost of Goods Sold",
        "Gross Profit", "\"Selling, General, and Admin Exp\"",
        "Operating Income before Depr",
        "Depreciation and Amortization",
        "Operating Profit", "Interest Expense",
        "Other Gains and Losses", "Pretax Income",
        "Income Tax Expense", "Net Income",
        };
    const char *names_2 [] = { " ", " ", "COGS", " ",
        "SG&A", "Operating Income before Depreciation"};
    
    size_t len_1 = strlen(names_1[line_num]);
    size_t len_2 = strlen(names_2[line_num]);
    
    if( !strncmp(names_1[line_num], line, len_1) )
    {
        strncpy(name, names_1[line_num], len_1);
        return len_1 + 1;
    }
    else if( !strncmp(names_2[line_num], line, len_2) )
    {
        strncpy(name, names_2[line_num], len_2);
        return len_2 + 1;
    }
    return -1;
}

[toc] | [prev] | [next] | [standalone]

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | comp.lang.c

csiph-web