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 1 of 3  [1] 2 3  Next page →

#101741 — Program to check for valid names

Hello!
 
Given the amount of valuable advice I get here, and the amount
and quality of knowledge I acquire here, I have decided to run the
following piece by you to get your opinion on the structure and the
logic. I am afraid to loose those tiny seeds of improvement, if I 
go on writing my little program completely on my own, only occasionally 
asking for help with some pieces.

Please, take a look at this little part of the program. 
 
 The goal of this little part is to read all lines from a file, and
 upon reading each line check if it doesn't start with a number,
 and, if it doesn't, then copy each valid character into another
 buffer by calling function check_input_names; this function should
 check the buffer for valid characters, and if there are any, copy
them into another place, and then call the function, which checks
if copied names can be found in the array of valid names; at the end
return the address of the first invalid character in the buffer (I need it
to be so).
I use some debugging in the program, sorry for that, but it helps me. 
Please, let me know if I have to delete all those when posting the code
here.
 Valid characters are: alphabetical ones, commas, double
 quotation marks, spaces (not any whitespace, but literally space).
 Here is the input (many of you have seen this many times already,
 so I apologize for posting it again):
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

Output (part of it):
****************************
No names on line 0
Line 0 number 1.00
****************************
Check_names: invalid name or empty input at line 1
****************************
No names on line 2
Line 2 number -2.00
****************************
No names on line 3
Line 3 number 9.00
****************************
No names on line 4
Line 4 number 2.00
****************************
****************************
No names on line 6
Line 6 number 7.00
****************************
No names on line 7
Line 7 number 3.00
****************************
No names on line 8
Line 8 number 2.00
****************************
No names on line 9
Line 9 number 1.00
****************************
Ptr array function end: Years
Not_converted 2011,2012,2013,2014,2015

****************************
****************************
No names on line 12
Line 12 number 654.00
****************************
****************************
Check_names: invalid name or empty input at line 14
****************************
Ptr array function end: "Selling, General, and Admin Exp"
Not_converted 254,271,364,454,576%$@

****************************
Program:
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>
#include <stdbool.h>
#include <errno.h>

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

bool get_names(FILE *input_file);
char *check_input_names(char *buffer, size_t line);
bool compare_names(char *buffer);

int main(int argc, char *argv[])
{
    //Check for correct number of arguments
    if(argc > 2)
    {
        fputs("Sorry, the program processes only one file.\n", stderr);
        return EXIT_FAILURE;
    }
    //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 )
    {
        fprintf(stderr, "%s - %s\n", data_file, errno ? strerror(errno) : "Open failure!");
        return EXIT_FAILURE;
    }
    
    if( !get_names(fp) )
    {
        fclose(fp);
        return EXIT_FAILURE;
    }
    fclose(fp);
    return EXIT_SUCCESS;
}

/** Function to find if file contains valid names **/
bool get_names (FILE *input_file)
{
    char *buffer = malloc(BUFFER_SIZE);
    if(!buffer)
    {
        printf("Error! memory not allocated.");
        return false;
    }
    size_t line = 0;
    
    //Parse file data, check if line contains valid names
    while( fgets(buffer, BUFFER_SIZE, input_file) != NULL)
    {
        char *not_converted = NULL;
        double number = strtod(buffer, &not_converted);
        puts("****************************");
        if(not_converted == buffer)
        {
            if( (not_converted = check_input_names(not_converted, line)) )
                printf("Not_converted %s\n", not_converted);
        }
        else
            printf("No names on line %zu\n", line);
        
        //DEBUG
        if(number != 0)
            printf("Line %zu number %.2f\n", line, number);
        
        line++;
    }
    if (ferror(input_file))
    {
        fprintf(stderr, "IO error after line %zu.\n", line);
        free(buffer);
        return false;
    }
    free(buffer);
    return true;
}

/** Function to check if input contains valid names **/
char *check_input_names(char *buffer, size_t line)
{
    //Skip beginning spaces
    while(isspace(*buffer))
        buffer++;
    
    char *ptr = malloc(BUFFER_SIZE);
    if(!ptr)
    {
        fprintf(stderr, "Error! Check_names: memory not allocated\n.");
        return NULL;
    }
    //Save the beginning address
    char *beginning = ptr;
    int count = 0;
    while(isalpha(*buffer) || isspace(*buffer)
          || *buffer == '"'|| *buffer == '&'
          || *buffer == ',')
    {
        *ptr++ = *buffer++;
        count++;
    }
    //No valid characters found?
    if(count == 0)
    {
        fprintf(stderr, "Check_names: invalid name or empty input at line %zu\n", line);
        free(ptr);
        return NULL;
    }
    if(*(ptr-1) == ',')
        ptr--;
    *ptr = '\0';
    //Set ptr back to the beginning addresss
    ptr = beginning;
    //Compare found names to required ones
    if(compare_names(ptr))
    {
        printf("Ptr array function end: %s\n", ptr);
        free(ptr);
        return buffer;
    }
    free(ptr);
    return NULL;
}

/** Function to compare names to 
    the required set of names 
 **/
bool compare_names(char *buffer)
{
    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 Depriciation",
        "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(!strcmp(names[i], buffer))
            return true;
        i++;
    }
    return false;
}

Thank you! 

[toc] | [next] | [standalone]

#101766

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

> Given the amount of valuable advice I get here, and the amount
> and quality of knowledge I acquire here, I have decided to run the
> following piece by you to get your opinion on the structure and the
> logic. I am afraid to loose those tiny seeds of improvement, if I 
> go on writing my little program completely on my own, only occasionally 
> asking for help with some pieces.
>
> Please, take a look at this little part of the program. 
>  
>  The goal of this little part is to read all lines from a file, and
>  upon reading each line check if it doesn't start with a number,

Why does that matter to you?

>  and, if it doesn't, then copy each valid character into another
>  buffer by calling function check_input_names; this function should
>  check the buffer for valid characters, and if there are any, copy
> them into another place, and then call the function, which checks
> if copied names can be found in the array of valid names; at the end
> return the address of the first invalid character in the buffer (I need it
> to be so).

This is not the right way to perform the overall goal of checking for a
valid name for some fixed list of names.

The key function to look up is strncmp and then you can avoid all the
copying of data.

<snip>
-- 
Ben.

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

#101772

On Saturday, January 14, 2017 at 6:02:08 PM UTC+3, Ben Bacarisse wrote:
> Alla _ <modelling.data@gmail.com> writes:
> 
> > Given the amount of valuable advice I get here, and the amount
> > and quality of knowledge I acquire here, I have decided to run the
> > following piece by you to get your opinion on the structure and the
> > logic. I am afraid to loose those tiny seeds of improvement, if I 
> > go on writing my little program completely on my own, only occasionally 
> > asking for help with some pieces.
> >
> > Please, take a look at this little part of the program. 
> >  
> >  The goal of this little part is to read all lines from a file, and
> >  upon reading each line check if it doesn't start with a number,
> 
> Why does that matter to you?
It does. I will extend that part further, to work with numbers. 
strtod turned out to be a great tool. 
> 
> >  and, if it doesn't, then copy each valid character into another
> >  buffer by calling function check_input_names; this function should
> >  check the buffer for valid characters, and if there are any, copy
> > them into another place, and then call the function, which checks
> > if copied names can be found in the array of valid names; at the end
> > return the address of the first invalid character in the buffer (I need it
> > to be so).
> 
> This is not the right way to perform the overall goal of checking for a
> valid name for some fixed list of names.
> 
> The key function to look up is strncmp and then you can avoid all the
> copying of data.
Here is what is said about strncmp in the manual:
"The strncmp() function shall compare not more than n bytes "
I am not sure I understand why strncmp is more helpful than strcmp I use now.
I have an array of a certain set of names, each of which differs in length. 
I think I could take the maximum length for n in this case, but why do I need it?
 
I read a line; if this line begins with the valid character, then I need to check 
everything in the line before I hit the first comma, which is right before the first number. 
This is why I copy only valid characters, and then use strcmp to see if the 
name is valid. 
> 
> <snip>
> -- 
> Ben.

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

#101785

On Saturday, January 14, 2017 at 9:29:40 AM UTC-6, Alla _ wrote:
> "The strncmp() function shall compare not more than n bytes "
> I am not sure I understand why strncmp is more helpful than strcmp I use now.

There are a number of common patterns for representing strings (in the
language-agnostic sense of the term).  Among them:

1. A zero-terminated sequence of characters is held in a buffer of unknown
   size.

2. A zero-terminated sequence of characters is held in a buffer of known
   size which is at least one byte larger than the maximum string length.

3. A zero-padded seqeunce of characters is held in a buffer of known
   size which is exactly equal to the maximum string length.

Some of the string functions are designed for the first two kinds of
strings, and some are designed for the third.  The strncpy function is
mainly useful for converting data to the third firm; data of the second
or third form may be converted to the second by using memcpy and then
writing zero to the following byte of the destination.

The primary usage of strncmp is to compare two strings which are stored
in the third form and have the same maximum length; it can also be
wrapped with some additional code to handle comparisons involving strings
of two different kinds.

While the third form of string storage is less common today than it used
to be, it can still be quite helpful when dealing with many strings whose
maximum length is small power, especially if it's of two (e.g. 4, 8, or
16).

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

#101787

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

> On Saturday, January 14, 2017 at 6:02:08 PM UTC+3, Ben Bacarisse wrote:
>> Alla _ <modelling.data@gmail.com> writes:
>> 
>> > Given the amount of valuable advice I get here, and the amount
>> > and quality of knowledge I acquire here, I have decided to run the
>> > following piece by you to get your opinion on the structure and the
>> > logic. I am afraid to loose those tiny seeds of improvement, if I 
>> > go on writing my little program completely on my own, only occasionally 
>> > asking for help with some pieces.
>> >
>> > Please, take a look at this little part of the program. 
>> >  
>> >  The goal of this little part is to read all lines from a file, and
>> >  upon reading each line check if it doesn't start with a number,
>> 
>> Why does that matter to you?

> It does.

But why?  None of the data lines you care about have numbers at the
start so looking for one is a waste of time.  Remember the plan is not
to find things that are wrong but to look for what you need and anything
else (a failure to find what you need) is an error.

> I will extend that part further, to work with numbers. 
> strtod turned out to be a great tool.

The numbers follow the text in your format.  What you will remove this
code, expand it and put it somewhere else.  At the moment all it does is
confuse people like me who have seen the format you are trying to read.

>> >  and, if it doesn't, then copy each valid character into another
>> >  buffer by calling function check_input_names; this function should
>> >  check the buffer for valid characters, and if there are any, copy
>> > them into another place, and then call the function, which checks
>> > if copied names can be found in the array of valid names; at the end
>> > return the address of the first invalid character in the buffer (I need it
>> > to be so).
>> 
>> This is not the right way to perform the overall goal of checking for a
>> valid name for some fixed list of names.
>> 
>> The key function to look up is strncmp and then you can avoid all the
>> copying of data.
> Here is what is said about strncmp in the manual:
> "The strncmp() function shall compare not more than n bytes "
> I am not sure I understand why strncmp is more helpful than strcmp I
> use now.

I'll try a hint.  Here is a program that counts the number of lines that
start with "Aha!".  Before you read it, think hou you would do that...

#include <stdio.h>
#include <string.h>

int main(void)
{
     unsigned count = 0;
     char line[100];
     while (fgets(line, sizeof line, stdin)) {
          if (strncmp(line, "Aha!", 4) == 0)
               count++;
     }
     printf("%d\n", count);
     return 0;
}

<snip>
-- 
Ben.

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

#101945

On Saturday, January 14, 2017 at 9:29:18 PM UTC+3, Ben Bacarisse wrote:
> Alla _ <modelling.data@gmail.com> writes:
> 
> > On Saturday, January 14, 2017 at 6:02:08 PM UTC+3, Ben Bacarisse wrote:
> >> Alla _ <modelling.data@gmail.com> writes:
> >> 
> >> > Given the amount of valuable advice I get here, and the amount
> >> > and quality of knowledge I acquire here, I have decided to run the
> >> > following piece by you to get your opinion on the structure and the
> >> > logic. I am afraid to loose those tiny seeds of improvement, if I 
> >> > go on writing my little program completely on my own, only occasionally 
> >> > asking for help with some pieces.
> >> >
> >> > Please, take a look at this little part of the program. 
> >> >  
> >> >  The goal of this little part is to read all lines from a file, and
> >> >  upon reading each line check if it doesn't start with a number,
> >> 
> >> Why does that matter to you?
> 
> > It does.
> 
> But why?  None of the data lines you care about have numbers at the
> start so looking for one is a waste of time.  Remember the plan is not
> to find things that are wrong but to look for what you need and anything
> else (a failure to find what you need) is an error.
> 
> > I will extend that part further, to work with numbers. 
> > strtod turned out to be a great tool.
> 
> The numbers follow the text in your format.  What you will remove this
> code, expand it and put it somewhere else.  At the moment all it does is
> confuse people like me who have seen the format you are trying to read.
> 
> >> >  and, if it doesn't, then copy each valid character into another
> >> >  buffer by calling function check_input_names; this function should
> >> >  check the buffer for valid characters, and if there are any, copy
> >> > them into another place, and then call the function, which checks
> >> > if copied names can be found in the array of valid names; at the end
> >> > return the address of the first invalid character in the buffer (I need it
> >> > to be so).
> >> 
> >> This is not the right way to perform the overall goal of checking for a
> >> valid name for some fixed list of names.
> >> 
> >> The key function to look up is strncmp and then you can avoid all the
> >> copying of data.
> > Here is what is said about strncmp in the manual:
> > "The strncmp() function shall compare not more than n bytes "
> > I am not sure I understand why strncmp is more helpful than strcmp I
> > use now.
> 
> I'll try a hint.  Here is a program that counts the number of lines that
> start with "Aha!".  Before you read it, think hou you would do that...
> 
> #include <stdio.h>
> #include <string.h>
> 
> int main(void)
> {
>      unsigned count = 0;
>      char line[100];
>      while (fgets(line, sizeof line, stdin)) {
>           if (strncmp(line, "Aha!", 4) == 0)
>                count++;
>      }
>      printf("%d\n", count);
>      return 0;
> }
I am truly sorry, but I don't understand. I have 13 names,
each has a different length. Each line I read from a file
has not only names, but also numbers. So I can't just 
use strncmp for each line, because:
- number of bytes is different for each element in the array
of names;
- even if I use something to determine the length of each 
name in the array and use that in strncmp, also using the 
loop to loop through each name in the array, the comparison to
0 will always be > or < and never = because the line contains
something else than just a name. This is why trying to follow
the method, I liked so much, of finding only what I need,
I copy only potentially valid input characters and then check
those. 

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

#101950

On 16/01/17 10:16, Alla _ wrote:
> On Saturday, January 14, 2017 at 9:29:18 PM UTC+3, Ben Bacarisse wrote:
>> Alla _ <modelling.data@gmail.com> writes:
>>
>>> On Saturday, January 14, 2017 at 6:02:08 PM UTC+3, Ben Bacarisse wrote:
>>>> Alla _ <modelling.data@gmail.com> writes:
>>>>
>>>>> Given the amount of valuable advice I get here, and the amount
>>>>> and quality of knowledge I acquire here, I have decided to run the
>>>>> following piece by you to get your opinion on the structure and the
>>>>> logic. I am afraid to loose those tiny seeds of improvement, if I
>>>>> go on writing my little program completely on my own, only occasionally
>>>>> asking for help with some pieces.
>>>>>
>>>>> Please, take a look at this little part of the program.
>>>>>
>>>>>  The goal of this little part is to read all lines from a file, and
>>>>>  upon reading each line check if it doesn't start with a number,
>>>>
>>>> Why does that matter to you?
>>
>>> It does.
>>
>> But why?  None of the data lines you care about have numbers at the
>> start so looking for one is a waste of time.  Remember the plan is not
>> to find things that are wrong but to look for what you need and anything
>> else (a failure to find what you need) is an error.
>>
>>> I will extend that part further, to work with numbers.
>>> strtod turned out to be a great tool.
>>
>> The numbers follow the text in your format.  What you will remove this
>> code, expand it and put it somewhere else.  At the moment all it does is
>> confuse people like me who have seen the format you are trying to read.
>>
>>>>>  and, if it doesn't, then copy each valid character into another
>>>>>  buffer by calling function check_input_names; this function should
>>>>>  check the buffer for valid characters, and if there are any, copy
>>>>> them into another place, and then call the function, which checks
>>>>> if copied names can be found in the array of valid names; at the end
>>>>> return the address of the first invalid character in the buffer (I need it
>>>>> to be so).
>>>>
>>>> This is not the right way to perform the overall goal of checking for a
>>>> valid name for some fixed list of names.
>>>>
>>>> The key function to look up is strncmp and then you can avoid all the
>>>> copying of data.
>>> Here is what is said about strncmp in the manual:
>>> "The strncmp() function shall compare not more than n bytes "
>>> I am not sure I understand why strncmp is more helpful than strcmp I
>>> use now.
>>
>> I'll try a hint.  Here is a program that counts the number of lines that
>> start with "Aha!".  Before you read it, think hou you would do that...
>>
>> #include <stdio.h>
>> #include <string.h>
>>
>> int main(void)
>> {
>>      unsigned count = 0;
>>      char line[100];
>>      while (fgets(line, sizeof line, stdin)) {
>>           if (strncmp(line, "Aha!", 4) == 0)
>>                count++;
>>      }
>>      printf("%d\n", count);
>>      return 0;
>> }
> I am truly sorry, but I don't understand. I have 13 names,
> each has a different length. Each line I read from a file
> has not only names, but also numbers. So I can't just
> use strncmp for each line, because:
> - number of bytes is different for each element in the array
> of names;

Yes, but you know (or can find out) how long they are.

> - even if I use something to determine the length of each
> name in the array and use that in strncmp, also using the
> loop to loop through each name in the array, the comparison to
> 0 will always be > or < and never = because the line contains
> something else than just a name.

Why? You're only looking at the right number of bytes on that line. If 
they are the right bytes, they are the right bytes, regardless of what 
follows. And if they are the wrong bytes, they are the wrong bytes, 
regardless of what follows. I really don't see your difficulty here.

> This is why trying to follow
> the method, I liked so much, of finding only what I need,
> I copy only potentially valid input characters and then check
> those.

Why would you need to /copy/ anything? I don't get it.

Why not just look for what you're looking for, and sing "hooray" if you 
find it, and write a diagnostic message if you don't?

-- 
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]

#101953

On Monday, January 16, 2017 at 2:40:19 PM UTC+3, Richard Heathfield wrote:
> On 16/01/17 10:16, Alla _ wrote:
> > On Saturday, January 14, 2017 at 9:29:18 PM UTC+3, Ben Bacarisse wrote:
> >> Alla _ <modelling.data@gmail.com> writes:
> >>
> >>> On Saturday, January 14, 2017 at 6:02:08 PM UTC+3, Ben Bacarisse wrote:
> >>>> Alla _ <modelling.data@gmail.com> writes:
> >>>>
> >>>>> Given the amount of valuable advice I get here, and the amount
> >>>>> and quality of knowledge I acquire here, I have decided to run the
> >>>>> following piece by you to get your opinion on the structure and the
> >>>>> logic. I am afraid to loose those tiny seeds of improvement, if I
> >>>>> go on writing my little program completely on my own, only occasionally
> >>>>> asking for help with some pieces.
> >>>>>
> >>>>> Please, take a look at this little part of the program.
> >>>>>
> >>>>>  The goal of this little part is to read all lines from a file, and
> >>>>>  upon reading each line check if it doesn't start with a number,
> >>>>
> >>>> Why does that matter to you?
> >>
> >>> It does.
> >>
> >> But why?  None of the data lines you care about have numbers at the
> >> start so looking for one is a waste of time.  Remember the plan is not
> >> to find things that are wrong but to look for what you need and anything
> >> else (a failure to find what you need) is an error.
> >>
> >>> I will extend that part further, to work with numbers.
> >>> strtod turned out to be a great tool.
> >>
> >> The numbers follow the text in your format.  What you will remove this
> >> code, expand it and put it somewhere else.  At the moment all it does is
> >> confuse people like me who have seen the format you are trying to read.
> >>
> >>>>>  and, if it doesn't, then copy each valid character into another
> >>>>>  buffer by calling function check_input_names; this function should
> >>>>>  check the buffer for valid characters, and if there are any, copy
> >>>>> them into another place, and then call the function, which checks
> >>>>> if copied names can be found in the array of valid names; at the end
> >>>>> return the address of the first invalid character in the buffer (I need it
> >>>>> to be so).
> >>>>
> >>>> This is not the right way to perform the overall goal of checking for a
> >>>> valid name for some fixed list of names.
> >>>>
> >>>> The key function to look up is strncmp and then you can avoid all the
> >>>> copying of data.
> >>> Here is what is said about strncmp in the manual:
> >>> "The strncmp() function shall compare not more than n bytes "
> >>> I am not sure I understand why strncmp is more helpful than strcmp I
> >>> use now.
> >>
> >> I'll try a hint.  Here is a program that counts the number of lines that
> >> start with "Aha!".  Before you read it, think hou you would do that...
> >>
> >> #include <stdio.h>
> >> #include <string.h>
> >>
> >> int main(void)
> >> {
> >>      unsigned count = 0;
> >>      char line[100];
> >>      while (fgets(line, sizeof line, stdin)) {
> >>           if (strncmp(line, "Aha!", 4) == 0)
> >>                count++;
> >>      }
> >>      printf("%d\n", count);
> >>      return 0;
> >> }
> > I am truly sorry, but I don't understand. I have 13 names,
> > each has a different length. Each line I read from a file
> > has not only names, but also numbers. So I can't just
> > use strncmp for each line, because:
> > - number of bytes is different for each element in the array
> > of names;
> 
> Yes, but you know (or can find out) how long they are.
> 
> > - even if I use something to determine the length of each
> > name in the array and use that in strncmp, also using the
> > loop to loop through each name in the array, the comparison to
> > 0 will always be > or < and never = because the line contains
> > something else than just a name.
> 
> Why? You're only looking at the right number of bytes on that line.
Ah, I see, indeed. I might have gotten the idea - have to write it out now.
Will be back. 
> If 
> they are the right bytes, they are the right bytes, regardless of what 
> follows. And if they are the wrong bytes, they are the wrong bytes, 
> regardless of what follows. I really don't see your difficulty here.
> 
> > This is why trying to follow
> > the method, I liked so much, of finding only what I need,
> > I copy only potentially valid input characters and then check
> > those.
> 
> Why would you need to /copy/ anything? I don't get it.
> 
> Why not just look for what you're looking for, and sing "hooray" if you 
> find it, and write a diagnostic message if you don't?
> 
> -- 
> 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]

#101955

On Monday, January 16, 2017 at 2:50:46 PM UTC+3, Alla _ wrote:
> On Monday, January 16, 2017 at 2:40:19 PM UTC+3, Richard Heathfield wrote:
> > On 16/01/17 10:16, Alla _ wrote:
> > > On Saturday, January 14, 2017 at 9:29:18 PM UTC+3, Ben Bacarisse wrote:
> > >> Alla _ <modelling.data@gmail.com> writes:
> > >>
> > >>> On Saturday, January 14, 2017 at 6:02:08 PM UTC+3, Ben Bacarisse wrote:
> > >>>> Alla _ <modelling.data@gmail.com> writes:
> > >>>>
> > >>>>> Given the amount of valuable advice I get here, and the amount
> > >>>>> and quality of knowledge I acquire here, I have decided to run the
> > >>>>> following piece by you to get your opinion on the structure and the
> > >>>>> logic. I am afraid to loose those tiny seeds of improvement, if I
> > >>>>> go on writing my little program completely on my own, only occasionally
> > >>>>> asking for help with some pieces.
> > >>>>>
> > >>>>> Please, take a look at this little part of the program.
> > >>>>>
> > >>>>>  The goal of this little part is to read all lines from a file, and
> > >>>>>  upon reading each line check if it doesn't start with a number,
> > >>>>
> > >>>> Why does that matter to you?
> > >>
> > >>> It does.
> > >>
> > >> But why?  None of the data lines you care about have numbers at the
> > >> start so looking for one is a waste of time.  Remember the plan is not
> > >> to find things that are wrong but to look for what you need and anything
> > >> else (a failure to find what you need) is an error.
> > >>
> > >>> I will extend that part further, to work with numbers.
> > >>> strtod turned out to be a great tool.
> > >>
> > >> The numbers follow the text in your format.  What you will remove this
> > >> code, expand it and put it somewhere else.  At the moment all it does is
> > >> confuse people like me who have seen the format you are trying to read.
> > >>
> > >>>>>  and, if it doesn't, then copy each valid character into another
> > >>>>>  buffer by calling function check_input_names; this function should
> > >>>>>  check the buffer for valid characters, and if there are any, copy
> > >>>>> them into another place, and then call the function, which checks
> > >>>>> if copied names can be found in the array of valid names; at the end
> > >>>>> return the address of the first invalid character in the buffer (I need it
> > >>>>> to be so).
> > >>>>
> > >>>> This is not the right way to perform the overall goal of checking for a
> > >>>> valid name for some fixed list of names.
> > >>>>
> > >>>> The key function to look up is strncmp and then you can avoid all the
> > >>>> copying of data.
> > >>> Here is what is said about strncmp in the manual:
> > >>> "The strncmp() function shall compare not more than n bytes "
> > >>> I am not sure I understand why strncmp is more helpful than strcmp I
> > >>> use now.
> > >>
> > >> I'll try a hint.  Here is a program that counts the number of lines that
> > >> start with "Aha!".  Before you read it, think hou you would do that...
> > >>
> > >> #include <stdio.h>
> > >> #include <string.h>
> > >>
> > >> int main(void)
> > >> {
> > >>      unsigned count = 0;
> > >>      char line[100];
> > >>      while (fgets(line, sizeof line, stdin)) {
> > >>           if (strncmp(line, "Aha!", 4) == 0)
> > >>                count++;
> > >>      }
> > >>      printf("%d\n", count);
> > >>      return 0;
> > >> }
> > > I am truly sorry, but I don't understand. I have 13 names,
> > > each has a different length. Each line I read from a file
> > > has not only names, but also numbers. So I can't just
> > > use strncmp for each line, because:
> > > - number of bytes is different for each element in the array
> > > of names;
> > 
> > Yes, but you know (or can find out) how long they are.
> > 
> > > - even if I use something to determine the length of each
> > > name in the array and use that in strncmp, also using the
> > > loop to loop through each name in the array, the comparison to
> > > 0 will always be > or < and never = because the line contains
> > > something else than just a name.
> > 
> > Why? You're only looking at the right number of bytes on that line.
> Ah, I see, indeed. I might have gotten the idea - have to write it out now.
> Will be back. 
<snip>
60 lines out ))) More will be out as well, when I get rid of my debugging
printfs.

Is it better?

#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>
#include <stdbool.h>
#include <errno.h>

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

bool get_names(FILE *input_file);
bool compare_names(char *buffer);

int main(int argc, char *argv[])
{
    //Check for correct number of arguments
    if(argc > 2)
    {
        fputs("Sorry, the program processes only one file.\n", stderr);
        return EXIT_FAILURE;
    }
    //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 )
    {
        fprintf(stderr, "%s - %s\n", data_file, errno ? strerror(errno) : "Open failure!");
        return EXIT_FAILURE;
    }
    
    if( !get_names(fp) )
    {
        fclose(fp);
        return EXIT_FAILURE;
    }
    fclose(fp);
    return EXIT_SUCCESS;
}

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

/** Function to compare names to
    the required set of names 
 **/
bool compare_names(char *buffer)
{
    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 Depriciation",
        "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], buffer, strlen(names[i])))
        {
            printf("Names[%zu] = %zu\n", i, strlen(names[i]));
            return true;
        }
        i++;
    }
    return false;
}

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

#101958

On 16/01/17 12:21, Alla _ wrote:
<snip>
> 60 lines out ))) More will be out as well, when I get rid of my debugging
> printfs.
>
> Is it better?

It has some problems.

> #include <stdio.h>
> #include <stdlib.h>
> #include <ctype.h>
> #include <string.h>
> #include <stdbool.h>
> #include <errno.h>
>
> #define BUFFER_SIZE 100
> #define INPUT_FILE "../files/ex_1.csv"
>
> bool get_names(FILE *input_file);

This function is poorly named. It claims to get names, but where are the 
names? It takes a FILE *, and returns a bool. No names in sight!

Nor does it offer any indication of whether the names are valid, except 
via a printf. Rather, it returns false if a system error occurred 
(malloc returned NULL or ferror returned non-0), and true otherwise.

> bool compare_names(char *buffer);

Again, this function is poorly named, although not quite as poorly as 
get_names.

> int main(int argc, char *argv[])
> {
>     //Check for correct number of arguments
>     if(argc > 2)
>     {
>         fputs("Sorry, the program processes only one file.\n", stderr);
>         return EXIT_FAILURE;
>     }
>     //Determine a file to use
>     const char *data_file = (argc == 2)? argv[1] : INPUT_FILE;
>     errno = 0;

errno is at best a clumsy tool. Personally, I'd rip it out completely.

>     FILE *fp = fopen(data_file, "r");
>     //Couldn't open the file?
>     if ( !fp )
>     {
>         fprintf(stderr, "%s - %s\n", data_file, errno ? strerror(errno) : "Open failure!");
>         return EXIT_FAILURE;
>     }
>
>     if( !get_names(fp) )
>     {
>         fclose(fp);
>         return EXIT_FAILURE;
>     }
>     fclose(fp);
>     return EXIT_SUCCESS;

The structure here is clumsy.

Why not do something like this:

int rc = EXIT_FAILURE;

FILE *fp = fopen(data_file, "r");
if(fp != NULL)
{
   if(get_names(fp))
   {
     rc = EXIT_SUCCESS;
   }
   fclose(fp);
}
else
{
   fprintf(stderr, "Couldn't open %s for reading.\n", data_file);
}

return rc;

Now you only have one fclose and one return.

> }
>
> /** Function to find if file contains valid names **/
> bool get_names (FILE *input_file)
> {
>     char *buffer = malloc(BUFFER_SIZE);

You never resize this memory, and it's only 100 bytes, so you can remove 
a point of failure and skip the need for a malloc/free pair by writing:

char buffer[BUFFER_SIZE] = "";

Of course, this means that you can't use 'buffer' (and, by the way, it 
isn't exactly a buffer --- 'line' would be a better name) to skip 
through the data with ++, but you could use

>     char *beginning = buffer;

for that instead, except that you'd want a better name, such as:

char line[MAX_LINE] = "";
char *token = line;

>     if(!buffer)
>     {
>         printf("Error! memory not allocated.");
>         return false;
>     }
>     size_t line = 0;
>
>     //Parse file data, check if line contains valid names
>     while( fgets(buffer, BUFFER_SIZE, input_file) != NULL)
>     {
>         while(isspace(*buffer))
>             buffer++;

If you took my advice above, you would change this to:

while(fgets(line, sizeof line, input_file) != NULL)
{
   token = buffer;
   while(isspace(*token))
   {
     ++token;
   }

and then you'd realise that you could put this into a function:

char *skipspace(char *s)
{
   while(isspace(*s))
   {
     ++s;
   }
   return s;
}

so that you could write:

char *token = skipspace(buffer);

And, if you know that you won't be changing the value pointed to by 
token, you can make skipspace take and return const, too.

>         puts("****************************");
>         if(compare_names(buffer))

(This, of course, would be token rather than buffer if you were 
following my change suggestions.)

>             printf("Success. Buffer %s\n", buffer);

You'd be better off writing debugging lines to stderr rather than 
stdout. Then you can easily separate them at runtime.

On the other hand, in the event of a successful run, this is the /only/ 
indication your program gives that it succeeded!

> /** Function to compare names to
>     the required set of names
>  **/
> bool compare_names(char *buffer)
> {
>     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 Depriciation",

You might want to check your spelling of 'Depreciation' in the above line.

>         "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], buffer, strlen(names[i])))
>         {
>             printf("Names[%zu] = %zu\n", i, strlen(names[i]));
>             return true;
>         }
>         i++;
>     }
>     return false;
> }

I would use memcmp rather than strncmp here. Also, you take strlen twice 
for the same string, and it turns out that this is just to print the 
length of the string, which seems unnecessary. Debug statement, presumably.

LINEAR SEARCH: Yes, folks, there are better ways. But they would be a 
distraction right now. Save something for next month, okay?

-- 
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]

#101959

On 16/01/17 12:56, Richard Heathfield wrote:
<snip>
>
> If you took my advice above, you would change this to:
>
> while(fgets(line, sizeof line, input_file) != NULL)
> {
>   token = buffer;

Sorry.
token = line;

>   while(isspace(*token))
>   {
>     ++token;
>   }
>
> and then you'd realise that you could put this into a function:
>
> char *skipspace(char *s)
> {
>   while(isspace(*s))
>   {
>     ++s;
>   }
>   return s;
> }
>
> so that you could write:
>
> char *token = skipspace(buffer);

Sorry.
char *token = skipspace(line);

-- 
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]

#101960

On Monday, January 16, 2017 at 3:56:44 PM UTC+3, Richard Heathfield wrote:
> On 16/01/17 12:21, Alla _ wrote:
> <snip>
> > 60 lines out ))) More will be out as well, when I get rid of my debugging
> > printfs.
> >
> > Is it better?
> 
> It has some problems.
><snip>
> 
> LINEAR SEARCH: Yes, folks, there are better ways. But they would be a 
> distraction right now. Save something for next month, okay?
> 
Still reading all the rest, but a short comment on this. I wanted to use a 
trie here, but then I thought that it's a way too much for this sort of task. 

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

#101961

On 16/01/17 13:03, Alla _ wrote:
> On Monday, January 16, 2017 at 3:56:44 PM UTC+3, Richard Heathfield wrote:
>> On 16/01/17 12:21, Alla _ wrote:
>> <snip>
>>> 60 lines out ))) More will be out as well, when I get rid of my debugging
>>> printfs.
>>>
>>> Is it better?
>>
>> It has some problems.
>> <snip>
>>
>> LINEAR SEARCH: Yes, folks, there are better ways. But they would be a
>> distraction right now. Save something for next month, okay?
>>
> Still reading all the rest, but a short comment on this. I wanted to use a
> trie here, but then I thought that it's a way too much for this sort of task.

Agreed. Stick to linear search for now, knowing full well that there are 
better ways that you can learn about later on.

-- 
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]

#101964

On Monday, January 16, 2017 at 3:56:44 PM UTC+3, Richard Heathfield wrote:
> On 16/01/17 12:21, Alla _ wrote:
> <snip>
> > 60 lines out ))) More will be out as well, when I get rid of my debugging
> > printfs.
> >
> > Is it better?
> 
> It has some problems.
Thank you very much for this detailed analysis. See my
answers/questions below.
> 
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <ctype.h>
> > #include <string.h>
> > #include <stdbool.h>
> > #include <errno.h>
> >
> > #define BUFFER_SIZE 100
> > #define INPUT_FILE "../files/ex_1.csv"
> >
> > bool get_names(FILE *input_file);
> 
> This function is poorly named. It claims to get names, but where are the 
> names? It takes a FILE *, and returns a bool. No names in sight!
Yes, I didn't bother with name this time because I simply have forgotten 
about it - it is only a transient version to be a scaled up. But I have changed
the name. Please, see at the end.
> 
> Nor does it offer any indication of whether the names are valid, except 
> via a printf. Rather, it returns false if a system error occurred 
> (malloc returned NULL or ferror returned non-0), and true otherwise.
> 
> > bool compare_names(char *buffer);
> 
> Again, this function is poorly named, although not quite as poorly as 
> get_names.
> 
> > int main(int argc, char *argv[])
> > {
> >     //Check for correct number of arguments
> >     if(argc > 2)
> >     {
> >         fputs("Sorry, the program processes only one file.\n", stderr);
> >         return EXIT_FAILURE;
> >     }
> >     //Determine a file to use
> >     const char *data_file = (argc == 2)? argv[1] : INPUT_FILE;
> >     errno = 0;
> 
> errno is at best a clumsy tool. Personally, I'd rip it out completely.
Let me leave as it is for now. As we have discussed with Ben when I was
doing exercises, I would like to get back to all this error handing thing 
later on. 
> 
> >     FILE *fp = fopen(data_file, "r");
> >     //Couldn't open the file?
> >     if ( !fp )
> >     {
> >         fprintf(stderr, "%s - %s\n", data_file, errno ? strerror(errno) : "Open failure!");
> >         return EXIT_FAILURE;
> >     }
> >
> >     if( !get_names(fp) )
> >     {
> >         fclose(fp);
> >         return EXIT_FAILURE;
> >     }
> >     fclose(fp);
> >     return EXIT_SUCCESS;
> 
> The structure here is clumsy.
> 
> Why not do something like this:
> 
> int rc = EXIT_FAILURE;
> 
> FILE *fp = fopen(data_file, "r");
> if(fp != NULL)
> {
>    if(get_names(fp))
>    {
>      rc = EXIT_SUCCESS;
>    }
>    fclose(fp);
> }
> else
> {
>    fprintf(stderr, "Couldn't open %s for reading.\n", data_file);
> }
> 
> return rc;
> 
> Now you only have one fclose and one return.
> 
Indeed. This is so much better. Done.
> > }
> >
> > /** Function to find if file contains valid names **/
> > bool get_names (FILE *input_file)
> > {
> >     char *buffer = malloc(BUFFER_SIZE);
> 
> You never resize this memory, 
Not yet, but... Please, forgive me but I am not changing this mallocing part. 

>and it's only 100 bytes, so you can remove 
> a point of failure and skip the need for a malloc/free pair by writing:
> 
> char buffer[BUFFER_SIZE] = "";
> 
Yes, this is exactly what I had before, but I have changed it to dynamic 
memory allocation. I would like it to be that way. I do not give up on using 
realloc. Let's not discuss it for now, please. 

> Of course, this means that you can't use 'buffer' (and, by the way, it 
> isn't exactly a buffer --- 'line' would be a better name) to skip 
> through the data with ++, but you could use
> 
> >     char *beginning = buffer;
> 
> for that instead, except that you'd want a better name, such as:
> 
> char line[MAX_LINE] = "";
> char *token = line;
> 
> >     if(!buffer)
> >     {
> >         printf("Error! memory not allocated.");
> >         return false;
> >     }
> >     size_t line = 0;
> >
> >     //Parse file data, check if line contains valid names
> >     while( fgets(buffer, BUFFER_SIZE, input_file) != NULL)
> >     {
> >         while(isspace(*buffer))
> >             buffer++;
> 
> If you took my advice above, you would change this to:
> 
> while(fgets(line, sizeof line, input_file) != NULL)
> {
>    token = buffer;
>    while(isspace(*token))
>    {
>      ++token;
>    }
> 
> and then you'd realise that you could put this into a function:
> 
> char *skipspace(char *s)
> {
>    while(isspace(*s))
>    {
>      ++s;
>    }
>    return s;
> }
> 
Yes, I do have this neat function in another version, and have not yet
put it here. Done. 
> so that you could write:
> 
> char *token = skipspace(buffer);
> 
> And, if you know that you won't be changing the value pointed to by 
> token, you can make skipspace take and return const, too.

I have not yet used this, therefore I am not fully aware of how to do it:
I declare line as a char *line. Then I pass line as a parameter to skip_spaces,
which in this case I can't declare as const char *skip_spaces(const char *line)
because I use skip_spaces in
line = skip_spaces(line); 
I can't assign const char * to a char*, correct? 

> 
> >         puts("****************************");
> >         if(compare_names(buffer))
> 
> (This, of course, would be token rather than buffer if you were 
> following my change suggestions.)
> 
> >             printf("Success. Buffer %s\n", buffer);
> 
> You'd be better off writing debugging lines to stderr rather than 
> stdout. Then you can easily separate them at runtime.
Sorry, please, repeat how can I do that - I mean how can I separate
these messages at the run time.
I will leave these printfs for now, because these are debugging things,
which I will delete going further. 
> 
> On the other hand, in the event of a successful run, this is the /only/ 
> indication your program gives that it succeeded!
> 
> > /** Function to compare names to
> >     the required set of names
> >  **/
> > bool compare_names(char *buffer)
> > {
> >     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 Depriciation",
> 
> You might want to check your spelling of 'Depreciation' in the above line.
> 
> >         "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], buffer, strlen(names[i])))
> >         {
> >             printf("Names[%zu] = %zu\n", i, strlen(names[i]));
> >             return true;
> >         }
> >         i++;
> >     }
> >     return false;
> > }
> 
> I would use memcmp rather than strncmp here. Also, you take strlen twice 
> for the same string, and it turns out that this is just to print the 
> length of the string, which seems unnecessary. Debug statement, presumably.
> 
> LINEAR SEARCH: Yes, folks, there are better ways. But they would be a 
> distraction right now. Save something for next month, okay?

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

#101966

On 16/01/17 14:40, Alla _ wrote:
<snip>
>>
>>> #include <stdio.h>
>>> #include <stdlib.h>
>>> #include <ctype.h>
>>> #include <string.h>
>>> #include <stdbool.h>
>>> #include <errno.h>
>>>
>>> #define BUFFER_SIZE 100
>>> #define INPUT_FILE "../files/ex_1.csv"
>>>
>>> bool get_names(FILE *input_file);
>>
>> This function is poorly named. It claims to get names, but where are the
>> names? It takes a FILE *, and returns a bool. No names in sight!
> Yes, I didn't bother with name this time because I simply have forgotten
> about it - it is only a transient version to be a scaled up. But I have changed
> the name. Please, see at the end.

Okay. Bear in mind, however, that when it comes to debugging, good name 
choices are half the battle. If you choose a misleading name, it can 
mislead you later on when you're trying to figure out what the hell has 
gone wrong.

>>>     errno = 0;
>>
>> errno is at best a clumsy tool. Personally, I'd rip it out completely.
> Let me leave as it is for now. As we have discussed with Ben when I was
> doing exercises, I would like to get back to all this error handing thing
> later on.

Well, okay. Ben may well have a different take on errno. For my part, I 
tend to ignore it completely, not least because Borland's implementation 
has a history of not supporting errno correctly. (Of course, they might 
have fixed it recently, but if they have I haven't heard about it.)

>>> /** Function to find if file contains valid names **/
>>> bool get_names (FILE *input_file)
>>> {
>>>     char *buffer = malloc(BUFFER_SIZE);
>>
>> You never resize this memory,
> Not yet, but... Please, forgive me but I am not changing this mallocing part.
[...]
> Yes, this is exactly what I had before, but I have changed it to dynamic
> memory allocation. I would like it to be that way. I do not give up on using
> realloc. Let's not discuss it for now, please.

Well, everything is open for discussion, but if you mean you have a plan 
in mind for this code where you will, later on, need this allocation to 
be dynamic, then of course that's fine. Comments are good for this:

char *line = malloc(BUFFER_SIZE); /* malloc because of future plans! */

as they may well forestall people from moaning about such things.

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.

<snip>

>> so that you could write:
>>
>> char *token = skipspace(buffer);
>>
>> And, if you know that you won't be changing the value pointed to by
>> token, you can make skipspace take and return const, too.
>
> I have not yet used this, therefore I am not fully aware of how to do it:
> I declare line as a char *line.

Yes. You called it 'buffer', but I think 'line' is a better name.

You would need to point it to some space (as you have in fact done in 
your existing code).

> Then I pass line as a parameter to skip_spaces,

Right.

> which in this case I can't declare as const char *skip_spaces(const char *line)
> because I use skip_spaces in
> line = skip_spaces(line);
> I can't assign const char * to a char*, correct?

That's correct, yes. Which is why I mentioned it as an aside, rather 
than as a firm suggestion.

>>>             printf("Success. Buffer %s\n", buffer);
>>
>> You'd be better off writing debugging lines to stderr rather than
>> stdout. Then you can easily separate them at runtime.
> Sorry, please, repeat how can I do that - I mean how can I separate
> these messages at the run time.

Let's say you have printf for your program output (the stuff the program 
spec says must be produced), and fprintf(stderr for your debugging output.

Then you can do this:

./alla_program_name input_file.csv > outputfile

and all the printfs will be written to the file (stdout is being 
redirected), but all your debug statements will appear on the console, 
not in the file.

If you'd rather see the output, and write the debug to a file, you can 
do that easily:

./alla_program_name input_file.csv 2> debugfile

Now you will see the program output, and the debug messages will go to 
debugfile

Or you can redirect them both:

./alla_program_name input_file.csv > outputfile 2> debugfile

> I will leave these printfs for now, because these are debugging things,
> which I will delete going further.

If you keep program output on stdout and debug output on stderr, you 
don't have to delete *anything*!

-- 
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]

#101975

On Monday, January 16, 2017 at 6:03:56 PM UTC+3, Richard Heathfield wrote:
> On 16/01/17 14:40, Alla _ wrote:
> <snip>
<snip>
> 
> >>> /** Function to find if file contains valid names **/
> >>> bool get_names (FILE *input_file)
> >>> {
> >>>     char *buffer = malloc(BUFFER_SIZE);
> >>
> >> You never resize this memory,
> > Not yet, but... Please, forgive me but I am not changing this mallocing part.
> [...]
> > Yes, this is exactly what I had before, but I have changed it to dynamic
> > memory allocation. I would like it to be that way. I do not give up on using
> > realloc. Let's not discuss it for now, please.
> 
> Well, everything is open for discussion, but if you mean you have a plan 
> in mind for this code where you will, later on, need this allocation to 
> be dynamic, then of course that's fine. Comments are good for this:
> 
> char *line = malloc(BUFFER_SIZE); /* malloc because of future plans! */
> 
> as they may well forestall people from moaning about such things.
> 
> 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, because skip_spaces
returns the address of the first after-space character. This is why I am 
using the first version.
> <snip>
<snip>
> 
> >>>             printf("Success. Buffer %s\n", buffer);
> >>
> >> You'd be better off writing debugging lines to stderr rather than
> >> stdout. Then you can easily separate them at runtime.
> > Sorry, please, repeat how can I do that - I mean how can I separate
> > these messages at the run time.
> 
> Let's say you have printf for your program output (the stuff the program 
> spec says must be produced), and fprintf(stderr for your debugging output.
> 
> Then you can do this:
> 
> ./alla_program_name input_file.csv > outputfile
> 
> and all the printfs will be written to the file (stdout is being 
> redirected), but all your debug statements will appear on the console, 
> not in the file.
> 
> If you'd rather see the output, and write the debug to a file, you can 
> do that easily:
> 
> ./alla_program_name input_file.csv 2> debugfile
> 
> Now you will see the program output, and the debug messages will go to 
> debugfile
> 
> Or you can redirect them both:
> 
> ./alla_program_name input_file.csv > outputfile 2> debugfile
> 
> > I will leave these printfs for now, because these are debugging things,
> > which I will delete going further.
> 
> If you keep program output on stdout and debug output on stderr, you 
> don't have to delete *anything*!

This is truly great ) I loved it, it works, it do have a file with all those
debugging messages in it, and nothing on the console, if I don't need
it ) How nice. I love these little tricks. Just a short note, without any
intention to trigger a discussion, but it would be even more interesting
to learn how these tricks are written and created. Please, do not respond
- this will truly start a huge new topic; not for now. Among numerous, 
not yet even opened, books I have this one:
http://www.charlespetzold.com/code/
Maybe one day I will get to reading it )

So, I assume the number 2 refers to a secondary "place" for the output, 
which is stderr for a compiler, which stdout is the first one, 
in layman terms. Correct?

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

#101978

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.

So: WHY do you think this?

> So, I assume the number 2 refers to a secondary "place" for the output,
> which is stderr for a compiler, which stdout is the first one,
> in layman terms. Correct?

Pretty much, yes: 2> redirects the standard error stream, and an 
unadorned > is shorthand for 1> which redirects the standard output stream.

-- 
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]

#101982

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.
> 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";

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, 
because I have initially allocated a certain amount of memory, which 
call to free() will try to free; if line points to some place after the beginning,
it means I will try to free memory beyond the one I have allocated initially
(beyond the end of the addresses allocated from input line). 

> 

<snip>

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

#101985

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).

<snip>
-- 
Ben.

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

#101987

On 16/01/17 17:17, 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).

Ben, do you have a PhD in Succinctology? :-)

-- 
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]

Page 1 of 3  [1] 2 3  Next page →

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

csiph-web