asked on

Alternative approach

Dear Experts

I have this following piece of code snippet whch constructs two messages to be sent on the basis of two if conditions

1> in function links() , when if (pcod == 'D') , the code below gets executed and when (pcod == 'S') the code in that if condition gets executed at the same time.

2> In the for loop that is for (i = 0 ; (i < LINKS) && Pplinka[i] ; ++i) under the functional links(mylreqst.l_typ_cod, mylreqst.l_action, Pplinka[i]);
again if if (myrequest.l_typ_cod =='D') then the message is constructed in the code below to it and when if (myrequest.l_typ_cod =='S') the code below to it constructs another message.

for (i = 0 ; (i < LINKS) && Pplinka[i] ; ++i)

      {

/* Set the individual request fields. */

links(mylreqst.l_typ_cod, mylreqst.l_action, Pplinka[i]);

      if (myrequest.l_typ_cod =='D')

      {
                 lj(mylreqst.l_filler_1, &nullstr, 2) ;

lj(mylreqst.l_filler_2, &nullstr, 11) ;
if (fwrite((char *)(&mylreqst), SIZE_LINKREQST, 1, pf) != 1)
return( ffmt_panic("fwrite(trans file)") ) ;
written += SIZE_LINKREQST ;
}


      if (myrequest.l_typ_cod =='S')

      {

      lj(mylreqst.l_filler_1, &nullstr, 2) ;
lj(mylreqst.l_filler_2, &nullstr, 11) ;
       /* Write a request segment. */
if (fwrite((char *)(&mylreqst), SIZE_LINKREQST, 1, pf) != 1)
return( ffmt_panic("fwrite(trans file)") ) ;
       written += SIZE_LINKREQST ;


      }

return (written) ;

}

static links(pcod,pcod1,paction,paction1, plinks)

char *pcod;

char *paction;

char *pcod1;

char *paction1;

LINK *plinks;

{

char act[1 + 1];

if (pcod == 'D')

{

if (*plink->c_link == EXI)

{

strcpy(act, FLNK);

}

else if (*plink->a_lnk == EXI)

{

strcpy(act, FLNK);

}

else if (*plink->r_lnk == EXI)

{

strcpy(act, FLNK);

}

lj(paction, act, 1) ;

}

}

if (pcod1 == 'S')

{

if (*plink->c_lnk == SES)

{

strcpy(act, RLNK);

}

else if (*plink->a_lnk == SES)

{

strcpy(act, RLNK);

}

else if (*plink->r_lnk == SES)

{

strcpy(act, RLNK);

}

lj(paction1, act, 1) ;

}

}

------------------------------------------------------------------------------------------------------------------------------------------------------
The above code snippet constructs two messages in cases when pcod1 == "D' and pcod1 == 'S'.
Is there any better way to use instead of two if conditions considering the code optimization point of view.
Thank you for your time

Ronan

rajeev_devin

Your code is a mess. Indent the code and then post.

ronan_40060

ASKER

Here is the proper identation

for (i = 0 ; (i < LINKS) && Pplinka[i] ; ++i)

{

/* Set the individual request fields. */
      links(mylreqst.l_typ_cod, mylreqst.l_action, Pplinka[i]);
            if (myrequest.l_typ_cod =='D')
                 {
                              lj(mylreqst.l_filler_1, &nullstr, 2) ;
lj(mylreqst.l_filler_2, &nullstr, 11) ;
if (fwrite((char *)(&mylreqst), SIZE_LINKREQST, 1, pf) != 1)
return( ffmt_panic("fwrite(trans file)") ) ;
written += SIZE_LINKREQST ;
                        }
if (myrequest.l_typ_cod =='S')
                        {
                              lj(mylreqst.l_filler_1, &nullstr, 2) ;
                              lj(mylreqst.l_filler_2, &nullstr, 11) ;
                   if (fwrite((char *)(&mylreqst), SIZE_LINKREQST, 1, pf) != 1)
                              return( ffmt_panic("fwrite(trans file)") ) ;
                              written += SIZE_LINKREQST ;
                        }
                              return (written) ;
      }

static links(pcod,pcod1,paction,paction1, plinks)

      char *pcod;
      char *paction;
      char *pcod1;
      char *paction1;
      LINK *plinks;
            {
                  char act[1 + 1];
                        if (pcod == 'D')
                              {
                                    if (*plink->c_link == EXI)
                                          {
                                                strcpy(act, FLNK);
                                          }
                                    else if (*plink->a_lnk == EXI)
                                          {
                                                strcpy(act, FLNK);
                                          }
                                    else if (*plink->r_lnk == EXI)
                                          {
                                                strcpy(act, FLNK);
                                          }
                                    lj(paction, act, 1) ;
                              }

                        if (pcod1 == 'S')
                              {
                                    if (*plink->c_lnk == SES)
                                          {
                                                strcpy(act, RLNK);
                                          }
                                    else if (*plink->a_lnk == SES)
                                          {
                                                strcpy(act, RLNK);
                                          }

                                    else if (*plink->r_lnk == SES)
                                          {
                                                strcpy(act, RLNK);
                                          }
                                    lj(paction1, act, 1) ;
                              }

            }

PaulCaswell

These two look exactly the same:

if (myrequest.l_typ_cod =='D')
{
lj(mylreqst.l_filler_1, &nullstr, 2) ;
lj(mylreqst.l_filler_2, &nullstr, 11) ;
if (fwrite((char *)(&mylreqst), SIZE_LINKREQST, 1, pf) != 1)
return( ffmt_panic("fwrite(trans file)") ) ;
written += SIZE_LINKREQST ;
}
if (myrequest.l_typ_cod =='S')
{
lj(mylreqst.l_filler_1, &nullstr, 2) ;
lj(mylreqst.l_filler_2, &nullstr, 11) ;
if (fwrite((char *)(&mylreqst), SIZE_LINKREQST, 1, pf) != 1)
return( ffmt_panic("fwrite(trans file)") ) ;
written += SIZE_LINKREQST ;
}

If that is true (I may have missed something) then you can code it as:

if (myrequest.l_typ_cod =='S' || myrequest.l_typ_cod =='D')
{
lj(mylreqst.l_filler_1, &nullstr, 2) ;
lj(mylreqst.l_filler_2, &nullstr, 11) ;
if (fwrite((char *)(&mylreqst), SIZE_LINKREQST, 1, pf) != 1)
return( ffmt_panic("fwrite(trans file)") ) ;
written += SIZE_LINKREQST ;
}

Paul

PaulCaswell

The same technique can be used here:

if (*plink->c_link == EXI)
{
strcpy(act, FLNK);
}
else if (*plink->a_lnk == EXI)
{
strcpy(act, FLNK);
}
else if (*plink->r_lnk == EXI)
{
strcpy(act, FLNK);
}

becomes:

if (*plink->c_link == EXI ||*plink->a_lnk == EXI || *plink->r_lnk == EXI)
{
strcpy(act, FLNK);
}

and in the second place.

Once that is done you will see a third place.

Paul

ronan_40060

ASKER

Paul thanks for your inputs
if (myrequest.l_typ_cod =='D') and if (myrequest.l_typ_cod =='S') are not same
in links() function I have pcod and pcode1 and which can have values 'D' and 'S' respectively.
On the basis of those two values , the following piece of code will also get modified as

links(mylreqst.l_typ_cod,mylreqst.l_typ_cod1, mylreqst.l_action,mylreqst.l_action1, Pplinka[i]);

if (myrequest.l_typ_cod =='D')
{
lj(mylreqst.l_filler_1, &nullstr, 2) ;
lj(mylreqst.l_filler_2, &nullstr, 11) ;
if (fwrite((char *)(&mylreqst), SIZE_LINKREQST, 1, pf) != 1)
return( ffmt_panic("fwrite(trans file)") ) ;
written += SIZE_LINKREQST ;
}
if (myrequest.l_typ_cod1 =='S')
{
lj(mylreqst.l_filler_1, &nullstr, 2) ;
lj(mylreqst.l_filler_2, &nullstr, 11) ;
if (fwrite((char *)(&mylreqst), SIZE_LINKREQST, 1, pf) != 1)
return( ffmt_panic("fwrite(trans file)") ) ;
written += SIZE_LINKREQST ;
}

Please let me know what you think
thanks for your time and inputs
Ronan

ASKER CERTIFIED SOLUTION

PaulCaswell

membership

This solution is only available to members.

To access this solution, you must be a member of Experts Exchange.

Start Free Trial

grg99

Some suggestions:

the code would be safer with the structure:

if code == CODE1 callthis()
else if code == CODE2 callthat
else fprintf stderr, "BAD FUNCTION CODE: '%c', code

------------

ANd #define CODE1 and CODE2 of course. And don't mix the address of strings "A" and comparing chars ( == 'A'). Too much room for confusion there.

ronan_40060

ASKER

Paul thanks for all your inputs
I have used your method
void DoX ()
{
X
}

then you can do:

if(A)
{
DoX();
}
if(B)
{
DoX();
}

grg99 thank you too

Have a nice day.
Ronan