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

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

Optimizing the code: way to reduce if, else if-s within the loop?

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

Contents

  Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-21 08:52 -0800
    Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-21 09:35 -0800
    Re: Optimizing the code: way to reduce if, else if-s within the loop? Barry Schwarz <schwarzb@dqel.com> - 2017-01-21 10:28 -0800
      Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-21 23:49 -0800
      Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-22 00:29 -0800
        Re: Optimizing the code: way to reduce if, else if-s within the loop? Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-01-22 10:54 +0000
          Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-22 03:28 -0800
            Re: Optimizing the code: way to reduce if, else if-s within the loop? Keith Thompson <kst-u@mib.org> - 2017-01-22 07:44 -0800
              Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-22 08:24 -0800
              Re: Optimizing the code: way to reduce if, else if-s within the loop? Barry Schwarz <schwarzb@dqel.com> - 2017-01-22 10:43 -0800
                Re: Optimizing the code: way to reduce if, else if-s within the loop? Keith Thompson <kst-u@mib.org> - 2017-01-22 12:22 -0800
          Re: Optimizing the code: way to reduce if, else if-s within the loop? BartC <bc@freeuk.com> - 2017-01-22 11:33 +0000
            Re: Optimizing the code: way to reduce if, else if-s within the loop? Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-01-22 13:23 +0000
              Re: Optimizing the code: way to reduce if, else if-s within the loop? BartC <bc@freeuk.com> - 2017-01-22 13:28 +0000
                Re: Optimizing the code: way to reduce if, else if-s within the loop? Keith Thompson <kst-u@mib.org> - 2017-01-22 07:47 -0800
                  Re: Optimizing the code: way to reduce if, else if-s within the loop? BartC <bc@freeuk.com> - 2017-01-22 16:00 +0000
                    Re: Optimizing the code: way to reduce if, else if-s within the loop? David Brown <david.brown@hesbynett.no> - 2017-01-22 18:23 +0100
                      Re: Optimizing the code: way to reduce if, else if-s within the loop? "James R. Kuyper" <jameskuyper@verizon.net> - 2017-01-22 13:47 -0500
                    Re: Optimizing the code: way to reduce if, else if-s within the loop? Barry Schwarz <schwarzb@dqel.com> - 2017-01-22 10:43 -0800
                      Re: Optimizing the code: way to reduce if, else if-s within the loop? "James R. Kuyper" <jameskuyper@verizon.net> - 2017-01-22 13:53 -0500
                    Re: Optimizing the code: way to reduce if, else if-s within the loop? Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-01-22 18:57 +0000
                      Re: Optimizing the code: way to reduce if, else if-s within the loop? BartC <bc@freeuk.com> - 2017-01-22 19:18 +0000
                        Re: Optimizing the code: way to reduce if, else if-s within the loop? Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-01-22 21:54 +0000
          Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-23 02:41 -0800
            Re: Optimizing the code: way to reduce if, else if-s within the loop? Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-01-23 11:20 +0000
              Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-23 03:53 -0800
                Re: Optimizing the code: way to reduce if, else if-s within the loop? Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-01-23 12:12 +0000
      Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-22 03:24 -0800
        Re: Optimizing the code: way to reduce if, else if-s within the loop? Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-01-22 13:16 +0000
        Re: Optimizing the code: way to reduce if, else if-s within the loop? Barry Schwarz <schwarzb@dqel.com> - 2017-01-22 07:47 -0800
          Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-22 08:16 -0800
            Re: Optimizing the code: way to reduce if, else if-s within the loop? "James R. Kuyper" <jameskuyper@verizon.net> - 2017-01-22 11:54 -0500
              Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-22 11:23 -0800
            Re: Optimizing the code: way to reduce if, else if-s within the loop? Barry Schwarz <schwarzb@dqel.com> - 2017-01-22 11:08 -0800
              Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-22 22:19 -0800
              Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-22 22:41 -0800
                Re: Optimizing the code: way to reduce if, else if-s within the loop? Robert Wessel <robertwessel2@yahoo.com> - 2017-01-23 02:17 -0600
                Re: Optimizing the code: way to reduce if, else if-s within the loop? Barry Schwarz <schwarzb@dqel.com> - 2017-01-23 02:16 -0800
                  Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-23 03:59 -0800
                Re: Optimizing the code: way to reduce if, else if-s within the loop? Keith Thompson <kst-u@mib.org> - 2017-01-23 08:44 -0800
          Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-22 08:23 -0800
        Re: Optimizing the code: way to reduce if, else if-s within the loop? Barry Schwarz <schwarzb@dqel.com> - 2017-01-22 07:52 -0800
          Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-22 08:17 -0800
      Re: Optimizing the code: way to reduce if, else if-s within the loop? Richard Heathfield <rjh@cpax.org.uk> - 2017-01-23 17:11 +0000
        Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-23 10:35 -0800
          Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-23 10:38 -0800
          Re: Optimizing the code: way to reduce if, else if-s within the loop? BartC <bc@freeuk.com> - 2017-01-23 19:06 +0000
          Re: Optimizing the code: way to reduce if, else if-s within the loop? Barry Schwarz <schwarzb@dqel.com> - 2017-01-23 14:31 -0800
            Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-23 21:31 -0800
            Re: Optimizing the code: way to reduce if, else if-s within the loop? Alla _ <modelling.data@gmail.com> - 2017-01-24 01:56 -0800
              Re: Optimizing the code: way to reduce if, else if-s within the loop? Manfred <noname@invalid.add> - 2017-01-24 14:38 +0100
              Re: Optimizing the code: way to reduce if, else if-s within the loop? Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-01-24 13:57 +0000
    Re: Optimizing the code: way to reduce if, else if-s within the loop? BartC <bc@freeuk.com> - 2017-01-23 12:38 +0000
      Re: Optimizing the code: way to reduce if, else if-s within the loop? Ben Bacarisse <ben.usenet@bsb.me.uk> - 2017-01-23 15:08 +0000
    Re: Optimizing the code: way to reduce if, else if-s within the loop? m.labanowicz@gmail.com - 2017-01-23 07:08 -0800

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

#102336

On Sunday, January 22, 2017 at 6:47:47 PM UTC+3, Barry Schwarz wrote:
> On Sun, 22 Jan 2017 03:24:39 -0800 (PST), Alla _
> <modelling.data@gmail.com> wrote:
> 
> >  *((char*)dat_years + offset) = strtod(ptr, &not_converted);
> 
> Here is the line of code I suggested
>      *(double*)((char*)dat_years + offset) = strtod(...);
> 
> You chose to leave out a significant piece, apparently thinking it
> didn't matter.  Now you need to explain to yourself why both cast
> operators are essential AND why they are placed where they are.  Ben
> has already explained what happens when you omit the first one.  What
> would happen you omitted the second?  What would happen if you moved
> the second outside the parenthetical expression?  What would happen if
> you omitted both?
> 
> Copying code others generate is stenography.  Programming requires you
> to understand what the code is doing.
Never thought this is the case ... :D

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

#102330

On Sun, 22 Jan 2017 03:24:39 -0800 (PST), Alla _
<modelling.data@gmail.com> wrote:

>int find_struct_member(char *name)
>{
>    int offset;
>    if(!strcmp(name, "Sales"))
>    {
>        offset = offsetof(struct data, sales);
>        fprintf(stderr, "offset sales %i\n", offset);
>    }
>    else if (!strcmp(name, "Cost of Goods Sold") ||
>             !strcmp(name, "COGS") )
>    {
>        offset = offsetof(struct data, cs_cogs);
>        fprintf(stderr, "offset cs_cogs %i\n", offset);
>    }

<snip<

>    else
>        return -1;
>    return offset;
>}

In most of your other code, you initialize every variable you define,
even if you immediately assign a new value.  Here where it would do
some good, you don't.

Initialize offset to -1 and discard the final else and its return
statement.  The return offset statement is sufficient.

-- 
Remove del for email

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

#102335

On Sunday, January 22, 2017 at 6:52:09 PM UTC+3, Barry Schwarz wrote:
> On Sun, 22 Jan 2017 03:24:39 -0800 (PST), Alla _
> <modelling.data@gmail.com> wrote:
> 
> >int find_struct_member(char *name)
> >{
> >    int offset;
> >    if(!strcmp(name, "Sales"))
> >    {
> >        offset = offsetof(struct data, sales);
> >        fprintf(stderr, "offset sales %i\n", offset);
> >    }
> >    else if (!strcmp(name, "Cost of Goods Sold") ||
> >             !strcmp(name, "COGS") )
> >    {
> >        offset = offsetof(struct data, cs_cogs);
> >        fprintf(stderr, "offset cs_cogs %i\n", offset);
> >    }
> 
> <snip<
> 
> >    else
> >        return -1;
> >    return offset;
> >}
> 
> In most of your other code, you initialize every variable you define,
> even if you immediately assign a new value.  Here where it would do
> some good, you don't.
> 
> Initialize offset to -1 and discard the final else and its return
> statement.  The return offset statement is sufficient.
Yes, indeed. I have initially set it to zero; then changed it 
back to nothing. Bad choice.

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

#102377

On 21/01/17 18:28, Barry Schwarz wrote:

<snip>

> Method 2:  If your system supports the offsetof macro

If there is a Purgatory for Usenauts, I think you just earned yourself a 
few hundred years. Alla's code now looks positively gruesome.

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

#102379

On Monday, January 23, 2017 at 8:11:18 PM UTC+3, Richard Heathfield wrote:
> On 21/01/17 18:28, Barry Schwarz wrote:
> 
> <snip>
> 
> > Method 2:  If your system supports the offsetof macro
> 
> If there is a Purgatory for Usenauts, I think you just earned yourself a 
> few hundred years. Alla's code now looks positively gruesome.
Well, but Barry has suggested a working approach that has helped;
all other approaches seem to be too sophisticated for me now. Yes,
the code isn't elegant, but I don't see any better way to do it now.
Ben has kindly agreed to help me learn a better approach once
I am done with the program.

Here is how that function looks now. Admit that I have made your 
evening now but showing you this piece :D

/** Function to determine a particular struct member **/
offsets find_struct_member(char *name, offsets *offset_ptr)
{
    //int offset = -1;
    if(!strcmp(name, "Sales"))
    {
        offset_ptr->offset_data = offsetof(data, sales);
        offset_ptr->offset_rates = offsetof(assumptions, gr_sales);
    }
    else if (!strcmp(name, "Cost of Goods Sold") ||
             !strcmp(name, "COGS") )
    {
        offset_ptr->offset_data = offsetof(struct data, cs_cogs);
        offset_ptr->offset_rates = offsetof(assumptions, av_cogs);
    }
    else if(!strcmp(name, "Gross Profit"))
    {
        offset_ptr->offset_data = offsetof(data, cs_gross_profit);
        offset_ptr->offset_rates = offsetof(assumptions, av_gross_profit);
    }
    else if (!strcmp(name, "\"Selling, General, and Admin Exp\"") ||
             !strcmp(name, "SG&A") )
    {
        offset_ptr->offset_data = offsetof(data, cs_sga);
        offset_ptr->offset_rates = offsetof(assumptions, av_sga);
    }
    else if (!strcmp(name, "Operating Income before Depr") ||
             !strcmp(name, "Operating Income before Depreciation") )
    {
        offset_ptr->offset_data = offsetof(data, cs_oper_inc);
        offset_ptr->offset_rates = offsetof(assumptions, av_oper_inc);
    }
    else if(!strcmp(name, "Depreciation and Amortization"))
    {
        offset_ptr->offset_data = offsetof(data, cs_dep_amor);
        offset_ptr->offset_rates = offsetof(assumptions, av_dep_amor);
    }
    else if(!strcmp(name, "Operating Profit"))
    {
        offset_ptr->offset_data = offsetof(data, cs_oper_prof);
        offset_ptr->offset_rates = offsetof(assumptions, av_oper_prof);
    }
    else if(!strcmp(name, "Interest Expense"))
    {
        offset_ptr->offset_data = offsetof(data, cs_int_exp);
        offset_ptr->offset_rates = offsetof(assumptions, av_int_exp);
    }
    else if(!strcmp(name, "Other Gains and Losses"))
    {
        offset_ptr->offset_data = offsetof(data, cs_gain_loss);
        offset_ptr->offset_rates = offsetof(assumptions, av_gain_loss);
    }
    else if(!strcmp(name, "Pretax Income"))
    {
        offset_ptr->offset_data = offsetof(data, cs_pretax_inc);
        offset_ptr->offset_rates = offsetof(assumptions, av_pretax_inc);
    }
    else if(!strcmp(name, "Income Tax Expense"))
    {
        offset_ptr->offset_data = offsetof(data, cs_tax_exp);
        offset_ptr->offset_rates = offsetof(assumptions, av_tax_exp);
    }
    else if(!strcmp(name, "Net Income"))
    {
        offset_ptr->offset_data = offsetof(data, cs_net_inc);
        offset_ptr->offset_rates = offsetof(assumptions, av_net_inc);
    }
    return *offset_ptr;
}

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

#102380

On Monday, January 23, 2017 at 9:35:22 PM UTC+3, Alla _ wrote:
> On Monday, January 23, 2017 at 8:11:18 PM UTC+3, Richard Heathfield wrote:
> > On 21/01/17 18:28, Barry Schwarz wrote:
> > 
> > <snip>
> > 
> > > Method 2:  If your system supports the offsetof macro
> > 
> > If there is a Purgatory for Usenauts, I think you just earned yourself a 
> > few hundred years. Alla's code now looks positively gruesome.
> Well, but Barry has suggested a working approach that has helped;
> all other approaches seem to be too sophisticated for me now. Yes,
> the code isn't elegant, but I don't see any better way to do it now.
> Ben has kindly agreed to help me learn a better approach once
> I am done with the program.
> 
> Here is how that function looks now. Admit that I have made your 
> evening now but showing you this piece :D
> 
Typo: not 'but', 'by'
<snip>

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

#102382

On 23/01/2017 18:35, Alla _ wrote:

> /** Function to determine a particular struct member **/
> offsets find_struct_member(char *name, offsets *offset_ptr)
> {

>     else if (!strcmp(name, "Operating Income before Depr") ||
>              !strcmp(name, "Operating Income before Depreciation") )
>     {
>         offset_ptr->offset_data = offsetof(data, cs_oper_inc);
>         offset_ptr->offset_rates = offsetof(assumptions, av_oper_inc);
>     }

What does a call look like to such a function? I'd imagine it might 
multiple calls to:

   find_struct_member("Operating Income before Depreciation",offset_ptr);

In which it wouldn't be hard to write that instead as multiple calls of:

   find_struct_member(offset_ptr, offsetof(data, cs_dep_amor),
                     offsetof(assumptions, av_dep_amor));

The contents of find_struct_member would then simplify to:

   void find_truct_member(offsets *offset_ptr,int dataoffset,
                        int ratesoffset) {
     offset_ptr->offset_data = dataoffset;
     offset_ptr->offset_rates = ratesoffset;
   }

If the call is part of the loop where the name part is picked from an 
array, then what you need is a routine to convert "Operating Income 
before Depreciation" to an index. Then everything is done with indices.

But I still can't see why you need to use names such as "Operating 
Income before Depreciation" at all. Just use an enum:

  enum { oper_inc, ...}

Then this is used to index everything, includes tables of offsets, 
tables of full names such as "Oper......Depreciation", or, if you want 
to keep the functions the same, you can just use oper_inc instead of the 
full title. Such descriptive titles should ever be for the user's 
benefit not for the internal workings of the code!

-- 
Bartc

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

#102386

On Mon, 23 Jan 2017 10:35:16 -0800 (PST), Alla _
<modelling.data@gmail.com> wrote:

>On Monday, January 23, 2017 at 8:11:18 PM UTC+3, Richard Heathfield wrote:
>> On 21/01/17 18:28, Barry Schwarz wrote:
>> 
>> <snip>
>> 
>> > Method 2:  If your system supports the offsetof macro
>> 
>> If there is a Purgatory for Usenauts, I think you just earned yourself a 
>> few hundred years. Alla's code now looks positively gruesome.

I recently apologized for this transgression.  Unfortunately she has
become enamored with it.

>Well, but Barry has suggested a working approach that has helped;
>all other approaches seem to be too sophisticated for me now. Yes,
>the code isn't elegant, but I don't see any better way to do it now.
>Ben has kindly agreed to help me learn a better approach once
>I am done with the program.
>
>Here is how that function looks now. Admit that I have made your 
>evening now but showing you this piece :D
>
>/** Function to determine a particular struct member **/
>offsets find_struct_member(char *name, offsets *offset_ptr)
>{
>    //int offset = -1;
>    if(!strcmp(name, "Sales"))
>    {
>        offset_ptr->offset_data = offsetof(data, sales);
>        offset_ptr->offset_rates = offsetof(assumptions, gr_sales);
>    }
>    else if (!strcmp(name, "Cost of Goods Sold") ||
>             !strcmp(name, "COGS") )
>    {
>        offset_ptr->offset_data = offsetof(struct data, cs_cogs);
>        offset_ptr->offset_rates = offsetof(assumptions, av_cogs);
>    }

PLEASE, go back and use the "switch statement" approach I suggested on
the 21st.

-- 
Remove del for email

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

#102393

On Tuesday, January 24, 2017 at 1:32:00 AM UTC+3, Barry Schwarz wrote:
> On Mon, 23 Jan 2017 10:35:16 -0800 (PST), Alla _
> <modelling.data@gmail.com> wrote:
> 
> >On Monday, January 23, 2017 at 8:11:18 PM UTC+3, Richard Heathfield wrote:
> >> On 21/01/17 18:28, Barry Schwarz wrote:
> >> 
> >> <snip>
> >> 
> >> > Method 2:  If your system supports the offsetof macro
> >> 
> >> If there is a Purgatory for Usenauts, I think you just earned yourself a 
> >> few hundred years. Alla's code now looks positively gruesome.
> 
> I recently apologized for this transgression.  Unfortunately she has
> become enamored with it.
You have a surprising proclivity to use wrong wording. I am not "enamored";
I do get enamored when I see elegant approaches, or neat tricks. Here
I am thankful for this approach, even though it is not elegant and doesn't 
look good, not enamored;  I has solved a huge problem I was
facing, without being to find a solution. Now I can access two structures
with the help of the offsetfof(), and I have only one function that sets
offsets for two structures when each line is read from the file. 

As to the offsetof(), as far as we already in this topic, where and how
it is normally used, if ever; what would be good circumstances to
apply this macro? 
<snip>

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

#102397

On Tuesday, January 24, 2017 at 1:32:00 AM UTC+3, Barry Schwarz wrote:
> On Mon, 23 Jan 2017 10:35:16 -0800 (PST), Alla _
> <modelling.data@gmail.com> wrote:
> 
><snip>
> PLEASE, go back and use the "switch statement" approach I suggested on
> the 21st.
Please, take a look at these functions: I have changed them, and now I use
switch statement. It works, I get the desired results, but it is rather big, too 
much vertical spread. 
Although setting members and then using switch statement looks a bit better 
then offsetof used within huge 'if-else if', please, explain to me why this is better:
offsetof() at least allowed  me to avoid using rather long
switch statements within 2 functions parse_numbers and assumptions_rates;
those two functions looked much more concise. 

/** Function to determine a particular struct member **/
int find_struct_member(char *name)
{
    int member = -1;
    if(!strcmp(name, "Sales"))
        member = 1;
    else if (!strcmp(name, "Cost of Goods Sold") ||
             !strcmp(name, "COGS") )
        member = 2;
    else if(!strcmp(name, "Gross Profit"))
        member = 3;
    else if (!strcmp(name, "\"Selling, General, and Admin Exp\"") ||
             !strcmp(name, "SG&A") )
        member = 4;
    else if (!strcmp(name, "Operating Income before Depr") ||
             !strcmp(name, "Operating Income before Depreciation") )
        member = 5;
    else if(!strcmp(name, "Depreciation and Amortization"))
        member = 6;
    else if(!strcmp(name, "Operating Profit"))
        member = 7;
    else if(!strcmp(name, "Interest Expense"))
        member = 8;
    else if(!strcmp(name, "Other Gains and Losses"))
        member = 9;
    else if(!strcmp(name, "Pretax Income"))
        member = 10;
    else if(!strcmp(name, "Income Tax Expense"))
        member = 11;
    else if(!strcmp(name, "Net Income"))
        member = 12;
    return member;
}

/** Function to parse numbers from each line **/
bool parse_numbers(char *line, int struct_member, data *dat_years, double *member_sum)
{
    char *ptr = line;
    char *not_converted = ptr;
    int count = 0;
    
    while(count < DATA_SIZE)
    {
        switch (struct_member)
        {
            case 1:
                dat_years->sales = strtod(ptr, &not_converted);
                break;
            case 2:
                dat_years->cs_cogs = strtod(ptr, &not_converted) / dat_years->sales;
                *member_sum += dat_years->cs_cogs;
                break;
            case 3:
                dat_years->cs_gross_profit = strtod(ptr, &not_converted) / dat_years->sales;
                *member_sum += dat_years->cs_gross_profit;
                break;
            case 4:
                dat_years->cs_sga = strtod(ptr, &not_converted) / dat_years->sales;
                *member_sum += dat_years->cs_sga;
                break;
            case 5:
                dat_years->cs_oper_inc = strtod(ptr, &not_converted) / dat_years->sales;
                *member_sum += dat_years->cs_oper_inc;
                break;
            case 6:
                break;
            case 7:
                dat_years->cs_oper_prof = strtod(ptr, &not_converted) / dat_years->sales;
                *member_sum += dat_years->cs_oper_prof;
            case 8:
                break;
            case 9:
                dat_years->cs_gain_loss = strtod(ptr, &not_converted) / dat_years->sales;
                *member_sum += dat_years->cs_gain_loss;
                break;
            case 10:
                dat_years->cs_pretax_inc = strtod(ptr, &not_converted) / dat_years->sales;
                *member_sum += dat_years->cs_pretax_inc;
                break;
            case 11:
                break;
            case 12:
                dat_years->cs_net_inc = strtod(ptr, &not_converted) / dat_years->sales;
                *member_sum += dat_years->cs_net_inc;
                break;
            default:
                break;
        }

        if (not_converted > ptr)
        {
            count++;
            dat_years += (count != DATA_SIZE);
            ptr = not_converted + (*not_converted != '\0');
        }
        else
        {
            fprintf(stderr, "Incorrect data on line %s at count %i\n", line, count);
            return false;
        }
    }
    return count == DATA_SIZE;
}

/** Function to compute assumptions **/
void assumptions_rates(int struct_member, data *dat_years, assumptions *rates, double member_sum)
{
    switch (struct_member)
    {
        case 1:
            rates->gr_sales = pow((dat_years[DATA_SIZE-1].sales/dat_years[0].sales), (1.0/DATA_SIZE));
            break;
        case 2:
            rates->av_cogs = member_sum / DATA_SIZE;
            break;
        case 3:
            rates->av_gross_profit = member_sum / DATA_SIZE;
            break;
        case 4:
            rates->av_sga = member_sum / DATA_SIZE;;
            break;
        case 5:
            rates->av_oper_inc = member_sum / DATA_SIZE;
            break;
        case 6:
            rates->av_dep_amor = 0;
            break;
        case 7:
            rates->av_oper_prof = member_sum / DATA_SIZE;
        case 8:
            rates->av_int_exp = 0;
            break;
        case 9:
            rates->av_gain_loss = member_sum / DATA_SIZE;
            break;
        case 10:
            rates->av_pretax_inc = member_sum / DATA_SIZE;
            break;
        case 11:
            rates->av_tax_exp = 0;
            break;
        case 12:
            rates->av_net_inc = member_sum / DATA_SIZE;
            break;
        default:
            break;
    }
}

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

#102400

On 1/24/2017 10:56 AM, Alla _ wrote:
> On Tuesday, January 24, 2017 at 1:32:00 AM UTC+3, Barry Schwarz wrote:
>> On Mon, 23 Jan 2017 10:35:16 -0800 (PST), Alla _
>> <modelling.data@gmail.com> wrote:
>>
>> <snip>
>> PLEASE, go back and use the "switch statement" approach I suggested on
>> the 21st.
> Please, take a look at these functions: I have changed them, and now I use
> switch statement. It works, I get the desired results, but it is rather big, too
> much vertical spread.
> Although setting members and then using switch statement looks a bit better
> then offsetof used within huge 'if-else if', please, explain to me why this is better:
> offsetof() at least allowed  me to avoid using rather long
> switch statements within 2 functions parse_numbers and assumptions_rates;
> those two functions looked much more concise.
My two cents on this: offsetof() deals with raw memory addresses, it 
looses all type information about members, and therefore it can make 
hard to find and debug errors if it is misused.
You don't have this problem in your current code, but your are rightly 
concerned too, because it is at least time consuming (and it can be 
error prone) to ensure that each and every if-else statement and case: 
instance does what it is expected to do.
I am not a particular fan of macros, but I think this is a case where 
they might result to be handy:
You may try to exploit the repetitive nature of such statements, and 
convert each of them into one-liners that only show the specific 
components, and hide the repetitive logic within the macro definition. 
See below.
Moreover, instead of using integer literals as member identifiers, 
probably an enum is better suited:
enum MemberID
{
   memInvalid = -1,
   memSales = 1,
   memCOGS,
   ...
}

>
> /** Function to determine a particular struct member **/
> int find_struct_member(char *name)
> {
>     int member = -1;
>     if(!strcmp(name, "Sales"))
>         member = 1;
here you might have a macro to replace each if-else statement with:
       IF_MEMBER_ID("Sales",              memSales)
       IF_MEMBER_ID("Cost of Goods Sold", memCOGS )
       IF_MEMBER_ID("COGS",               memCOGS )
       ...
>     else if (!strcmp(name, "Cost of Goods Sold") ||
>              !strcmp(name, "COGS") )
>         member = 2;
>     else if(!strcmp(name, "Gross Profit"))
>         member = 3;
>     else if (!strcmp(name, "\"Selling, General, and Admin Exp\"") ||
>              !strcmp(name, "SG&A") )
>         member = 4;
>     else if (!strcmp(name, "Operating Income before Depr") ||
>              !strcmp(name, "Operating Income before Depreciation") )
>         member = 5;
>     else if(!strcmp(name, "Depreciation and Amortization"))
>         member = 6;
>     else if(!strcmp(name, "Operating Profit"))
>         member = 7;
>     else if(!strcmp(name, "Interest Expense"))
>         member = 8;
>     else if(!strcmp(name, "Other Gains and Losses"))
>         member = 9;
>     else if(!strcmp(name, "Pretax Income"))
>         member = 10;
>     else if(!strcmp(name, "Income Tax Expense"))
>         member = 11;
>     else if(!strcmp(name, "Net Income"))
>         member = 12;
>     return member;
> }
>
> /** Function to parse numbers from each line **/
> bool parse_numbers(char *line, int struct_member, data *dat_years, double *member_sum)
> {
>     char *ptr = line;
>     char *not_converted = ptr;
>     int count = 0;
>
>     while(count < DATA_SIZE)
>     {
>         switch (struct_member)
>         {
here you might have a macro that replaces each case: instance like:
               CASE_YEARS_VALUE(memSales, dat_years->sales  );
               CASE_YEARS_VALUE(memCOGS,  dat_years->cs_cogs);
               You will have to move any non-repetitive logic out of the 
switch body, though.
               The strtod call may be moved before the switch statement 
(making use of a temporary variable of type double), and tested for 
success before assigning its result to any member.
>             case 1:
>                 dat_years->sales = strtod(ptr, &not_converted);
>                 break;
>             case 2:
>                 dat_years->cs_cogs = strtod(ptr, &not_converted) / dat_years->sales;
>                 *member_sum += dat_years->cs_cogs;
>                 break;
>             case 3:
>                 dat_years->cs_gross_profit = strtod(ptr, &not_converted) / dat_years->sales;
>                 *member_sum += dat_years->cs_gross_profit;
>                 break;
>             case 4:
>                 dat_years->cs_sga = strtod(ptr, &not_converted) / dat_years->sales;
>                 *member_sum += dat_years->cs_sga;
>                 break;
>             case 5:
>                 dat_years->cs_oper_inc = strtod(ptr, &not_converted) / dat_years->sales;
>                 *member_sum += dat_years->cs_oper_inc;
>                 break;
>             case 6:
>                 break;
>             case 7:
>                 dat_years->cs_oper_prof = strtod(ptr, &not_converted) / dat_years->sales;
>                 *member_sum += dat_years->cs_oper_prof;
>             case 8:
>                 break;
>             case 9:
>                 dat_years->cs_gain_loss = strtod(ptr, &not_converted) / dat_years->sales;
>                 *member_sum += dat_years->cs_gain_loss;
>                 break;
>             case 10:
>                 dat_years->cs_pretax_inc = strtod(ptr, &not_converted) / dat_years->sales;
>                 *member_sum += dat_years->cs_pretax_inc;
>                 break;
>             case 11:
>                 break;
>             case 12:
>                 dat_years->cs_net_inc = strtod(ptr, &not_converted) / dat_years->sales;
>                 *member_sum += dat_years->cs_net_inc;
>                 break;
>             default:
>                 break;
>         }
>
>         if (not_converted > ptr)
>         {
>             count++;
>             dat_years += (count != DATA_SIZE);
>             ptr = not_converted + (*not_converted != '\0');
>         }
>         else
>         {
>             fprintf(stderr, "Incorrect data on line %s at count %i\n", line, count);
>             return false;
>         }
>     }
>     return count == DATA_SIZE;
> }
>
> /** Function to compute assumptions **/
> void assumptions_rates(int struct_member, data *dat_years, assumptions *rates, double member_sum)
> {
>     switch (struct_member)
>     {
If you really need to have different expressions calculate the value to 
be assigned to the member, you may have the expression as an argument to 
the macro:
           CASE_ASSUMPTIONS_VALUE(
             memSales, rates->gr_sales, 
pow((dat_years[DATA_SIZE-1].sales/dat_years[0].sales), (1.0/DATA_SIZE)));
>         case 1:
>             rates->gr_sales = pow((dat_years[DATA_SIZE-1].sales/dat_years[0].sales), (1.0/DATA_SIZE));
>             break;
>         case 2:
>             rates->av_cogs = member_sum / DATA_SIZE;
>             break;
>         case 3:
>             rates->av_gross_profit = member_sum / DATA_SIZE;
>             break;
>         case 4:
>             rates->av_sga = member_sum / DATA_SIZE;;
>             break;
>         case 5:
>             rates->av_oper_inc = member_sum / DATA_SIZE;
>             break;
>         case 6:
>             rates->av_dep_amor = 0;
>             break;
>         case 7:
>             rates->av_oper_prof = member_sum / DATA_SIZE;
>         case 8:
>             rates->av_int_exp = 0;
>             break;
>         case 9:
>             rates->av_gain_loss = member_sum / DATA_SIZE;
>             break;
>         case 10:
>             rates->av_pretax_inc = member_sum / DATA_SIZE;
>             break;
>         case 11:
>             rates->av_tax_exp = 0;
>             break;
>         case 12:
>             rates->av_net_inc = member_sum / DATA_SIZE;
>             break;
>         default:
>             break;
>     }
> }
>
>
>

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

#102401

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

> On Tuesday, January 24, 2017 at 1:32:00 AM UTC+3, Barry Schwarz wrote:
>> On Mon, 23 Jan 2017 10:35:16 -0800 (PST), Alla _
>> <modelling.data@gmail.com> wrote:
>> 
>><snip>
>> PLEASE, go back and use the "switch statement" approach I suggested on
>> the 21st.
> Please, take a look at these functions: I have changed them, and now I use
> switch statement. It works, I get the desired results, but it is rather big, too 
> much vertical spread.

Yes, this is worse.  You now have a set of magic numbers in three places
none of which is linked to what it means.

Here's how you should do this sort of thing:

(a) Number the quantities with helpful names.  Do this with an enum:

  enum {
       Sales,
       COGS,
       GrossProfit,
       ...
       NumOfItems
  };

The extra "NoOfItems" is there so you have a count of how many
quantities are needed.

(b) To associate an input string with a quantity you need an array of
strings and matching quantities.

You now have a choice about how to identify the quantities.  You can
keep your struct and associate the name with an offset, or you can put
the quantities in an array and associate the name with an index.  I
would do the latter because it might lead to more flexible solutions
later:

  struct quantities {
       const char *name;
       int index;
  } quantity_tablep[] = {
       { "Sales",                 Sales },
       { "Cost of Goods Sold",    COGS  },
       { "COGS",                  COGS  },
       ...
  };

Note that there are two names that map to the same quantity index.  If
you where using offsets, you'd use size_of and put offsetof(..., sales)
(or whatever it is) in the second position.

(c) Write a function that scans the array for a matching name and
returns the corresponding index.  If you use indexes rather than
offsets, you can add

   InvalidIndex = -1,

(or some similar name) at the top of the enum and return that when the
string does not match a name.

<snip>
-- 
Ben.

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

#102371

On 21/01/2017 16:52, Alla _ wrote:

> /** Function to parse numbers from each line **/
> bool parse_numbers(char *line, struct data *dat_years, char *name)
> {
>     char *ptr = line;
>     char *not_converted = NULL;
>     int count = 0;
>
>     while(count < DATA_SIZE)
>     {
>         if(!strcmp(name, "Sales"))
>            dat_years->sales = strtod(ptr, &not_converted);
>
>         else if (!strcmp(name, "Cost of Goods Sold") ||
>                  !strcmp(name, "COGS") )
>            dat_years->cs_cogs = strtod(ptr, &not_converted);
>
>         else if(!strcmp(name, "Gross Profit"))
>            dat_years->cs_gross_profit = strtod(ptr, &not_converted);
>
>         else if (!strcmp(name, "\"Selling, General, and Admin Exp\"") ||
>                  !strcmp(name, "SG&A") )
>            dat_years->cs_sga = strtod(ptr, &not_converted);
>
>         else if (!strcmp(name, "Operating Income before Depr") ||
>                  !strcmp(name, "Operating Income before Depreciation") )
.......
>         if (not_converted > ptr)
>         {
>             count++;
>             dat_years += (count != DATA_SIZE);
>             ptr = not_converted + (*not_converted != '\0');
>         }
>         else
>        {
>             fprintf(stderr, "Incorrect data on line %s at count %i\n", line, count);
>             return false;
>         }
>     }

I'm trying to make sense of this as the code seems 'off' in so many ways.

The input is presumably an array of 'data' structs, each of which 
contains this set of assorted fields. (That it is not clear that this is 
an array, and not a pointer to one struct, is the language's fault not 
yours.)

'name' doesn't change that I can see, so the whole thing, and each loop 
iteration, is reading the same field. But the same field repeated on 
each input line? That doesn't sound right.

You are hard-coding all these field names, but presumably they exist 
somewhere else in the program? Or are they read from a file? If not, 
then you are repeating string data that exists elsewhere.

But I guess the names aren't read from a file, because otherwise how can 
you define a struct with exactly the right number of matching member 
names. Why would you need a list of field names as strings anyway? 
Wouldn't a set of enums be better? (And maybe the enum can index into an 
array that stores the value, rather than a named struct member.)

There is a long list of tests, but if 'name' matches one near the end, 
then you are repeatedly doing all those other compares for each iteration.

And you have alternatives for some of the names. In glimpses I've seen 
of what might be the rest of the program in other posts, you have a list 
of these main field names. And then you have a second list of 
alternatives which largely repeats the first!

Making three instances of most of the field names that have to be 
duplicated in the program (unless that was a version that replaces what 
is shown above; it's hard to keep track).

I can't see the whole program but it would be hard anyway to get an 
overview of what the program is trying to achieve. As there is a mass of 
detail.

What is the shape of the input file? What does the data structure look 
like? As it presumably is trying to read the file into the data 
structure. Where do these field names come into it?

-- 
Bartc

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

#102375

BartC <bc@freeuk.com> writes:

> On 21/01/2017 16:52, Alla _ wrote:
>
>> /** Function to parse numbers from each line **/
>> bool parse_numbers(char *line, struct data *dat_years, char *name)
>> {
>>     char *ptr = line;
>>     char *not_converted = NULL;
>>     int count = 0;
>>
>>     while(count < DATA_SIZE)
>>     {
>>         if(!strcmp(name, "Sales"))
>>            dat_years->sales = strtod(ptr, &not_converted);
>>
>>         else if (!strcmp(name, "Cost of Goods Sold") ||
>>                  !strcmp(name, "COGS") )
>>            dat_years->cs_cogs = strtod(ptr, &not_converted);
>>
>>         else if(!strcmp(name, "Gross Profit"))
>>            dat_years->cs_gross_profit = strtod(ptr, &not_converted);
>>
>>         else if (!strcmp(name, "\"Selling, General, and Admin Exp\"") ||
>>                  !strcmp(name, "SG&A") )
>>            dat_years->cs_sga = strtod(ptr, &not_converted);
>>
>>         else if (!strcmp(name, "Operating Income before Depr") ||
>>                  !strcmp(name, "Operating Income before Depreciation") )
> .......
>>         if (not_converted > ptr)
>>         {
>>             count++;
>>             dat_years += (count != DATA_SIZE);
>>             ptr = not_converted + (*not_converted != '\0');
>>         }
>>         else
>>        {
>>             fprintf(stderr, "Incorrect data on line %s at count %i\n", line, count);
>>             return false;
>>         }
>>     }
>
> I'm trying to make sense of this as the code seems 'off' in so many ways.
>
> The input is presumably an array of 'data' structs, each of which
> contains this set of assorted fields.

No, that's not how the input is structured -- at least not how I
interpret your words.  In effect Alla is transposing the matrix that I
think you are assuming.  The columns are years and the rows have these
data names.

<snip>
> What is the shape of the input file? What does the data structure look
> like? As it presumably is trying to read the file into the data
> structure. Where do these field names come into it?

It's been posted several times.  For example:

  6cbe3b83-1181-40fa-b007-b9913f92ef3f@googlegroups.com

-- 
Ben.

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

#102374

Example implementation based on sorted array approach:

:: [++main.c] ..........................
/*01*/
       #include <stddef.h>
       #include <stdio.h>
       #include <stdlib.h>
       #include <string.h>
/*06*/
/*07*/ typedef struct data {
/*08*/   int     year;
/*09*/   double  sales;
/*10*/   double  cs_cogs;
/*11*/   double  cs_gross_profit;
/*12*/   double  cs_sga;
/*13*/   double  cs_oper_inc;
/*14*/   double  cs_dep_amor;
/*15*/   double  cs_oper_prof;
/*16*/   double  cs_int_exp;
/*17*/   double  cs_gain_loss;
/*18*/   double  cs_pretax_inc;
/*19*/   double  cs_tax_exp;
/*20*/   double  cs_net_inc;
/*21*/ } data;
/*22*/
/*23*/ typedef struct {
/*24*/   char const * name;
/*25*/   size_t       offs;
/*26*/ } data_ElemInfo;
/*27*/
/*28*/ static data_ElemInfo const DATA_ELEMINFO [] = {
/*29*/   /* Array has to be sorted with 'strcmp' */
/*30*/   { "Cost of Goods Sold"                   , offsetof(data, cs_cogs        ) },
/*31*/   { "Depreciation and Amortization"        , offsetof(data, cs_dep_amor    ) },
/*32*/   { "Gross Profit"                         , offsetof(data, cs_gross_profit) },
/*33*/   { "Income Tax Expense"                   , offsetof(data, cs_tax_exp     ) },
/*34*/   { "Interest Expense"                     , offsetof(data, cs_int_exp     ) },
/*35*/   { "Net Income"                           , offsetof(data, cs_net_inc     ) },
/*36*/   { "Operating Income before Depr"         , offsetof(data, cs_oper_inc    ) },
/*37*/   { "Operating Income before Depreciation" , offsetof(data, cs_oper_inc    ) },
/*38*/   { "Operating Profit"                     , offsetof(data, cs_oper_prof   ) },
/*39*/   { "Other Gains and Losses"               , offsetof(data, cs_gain_loss   ) },
/*40*/   { "Pretax Income"                        , offsetof(data, cs_pretax_inc  ) },
/*41*/   { "SG&A"                                 , offsetof(data, cs_sga         ) },
/*42*/   { "Sales"                                , offsetof(data, sales          ) },
/*43*/   { "Selling, General, and Admin Exp"      , offsetof(data, cs_sga         ) }
/*44*/ };
/*45*/
/*46*/ static data_ElemInfo const * data_ElemInfoGet(char const * name)
/*47*/ {
/*48*/   data_ElemInfo const * begin = &DATA_ELEMINFO[0];
/*49*/   int count = (int)(sizeof(DATA_ELEMINFO) / sizeof(DATA_ELEMINFO[0]));
/*50*/   while (count > 0) {
/*51*/     int idx = count / 2;
/*52*/     int cmp = strcmp(name, begin[idx].name);
/*53*/     if (!cmp) {
/*54*/       return begin + idx;
/*55*/     }
/*56*/     if (cmp < 0) {
/*57*/       count = idx;
/*58*/     }
/*59*/     else {
/*60*/       begin += idx + 1;
/*61*/       count -= idx + 1;
/*62*/     }
/*63*/   }
/*64*/   return NULL;
/*65*/ }
/*66*/
/*67*/ int main(int argc, char * argv [])
/*68*/ {
/*69*/   while (--argc > 0) {
/*70*/     char const * name = *(++argv);
/*71*/     data_ElemInfo const * de = data_ElemInfoGet(name);
/*72*/     printf("\"%s\" --> ", name);
/*73*/     if (de) {
/*74*/       printf("offs=%lu\n", de->offs);
/*75*/     } else {
/*76*/       printf("!not found!\n");
/*77*/     }
/*78*/   }
/*79*/
/*80*/   return EXIT_SUCCESS;
/*81*/ }
/*82*/
:: [--main.c] ..........................

$ gcc main.c -o main.exe

$ ./main.exe "test" "Cost of Goods Sold" "SG&A" "Selling, General, and Admin Exp"
"test" --> !not found!
"Cost of Goods Sold" --> offs=16
"SG&A" --> offs=32
"Selling, General, and Admin Exp" --> offs=32

Cheers

-- 
Maciej Labanowicz

[toc] | [prev] | [standalone]

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

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

csiph-web