We help IT Professionals succeed at work.

We've partnered with Certified Experts, Carl Webster and Richard Faulkner, to bring you two Citrix podcasts. Learn about 2020 trends and get answers to your biggest Citrix questions!Listen Now

x

Alternative approach

on
Medium Priority
182 Views
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. */

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

}

char      *pcod;

char      *paction;

char    *pcod1;

char    *paction1;

{

char      act[1 + 1];

if (pcod == 'D')

{

{

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
Comment
Watch Question

View Solution Only

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

Commented:
Here is the proper identation

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

{

/* Set the individual request fields. */
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) ;
}

char      *pcod;
char      *paction;
char    *pcod1;
char    *paction1;
{
char act[1 + 1];
if (pcod == 'D')
{
{
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) ;
}

}

Commented:
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

Commented:
The same technique can be used here:

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

becomes:

{
strcpy(act, FLNK);
}

and in the second place.

Once that is done you will see a third place.

Paul

Commented:
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

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
Commented:
Hi ronan_40060,

>>...are not same
Ah! I missed that! Sorry!

Still, the essence of what I suggested is:

if(A)
{
X
}
if(B)
{
X
}

can always be meged into:

if( A || B )
{
X
}

unless you want X to be done twice if both are true. In which case you'd be better to use something like:

void DoX ()
{
X
}

then you can do:

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

Do you see?

I'm suggesting you try to turn the contents of the two code blocks into one separate function and call it. At least there is then ony one copy of the code instead of two.

You will often find that a name for the new function comes readily to mind and everything becomes much clearer.

Paul

Not the solution you were looking for? Getting a personalized solution is easy.

Commented:
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.

Commented:
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
Access more of Experts Exchange with a free account
Thanks for using Experts Exchange.

Limited access with a free account allows you to:

• View three pieces of content (articles, solutions, posts, and videos)
• Ask the experts questions (counted toward content limit)
• Customize your dashboard and profile