better code

A

arunix

hello all
i am here again. i write this code
how i can be better it?
Thannks in advance..


#include<iostream>
void sum_function();
bool accept();
int main()
{
accept();
// sum_function();
return 0;
}

void sum_function()
{
double d=2.2;
int i=7;
d= d+i;
std :: cout <<"The Sum of D & I is :- " << d
<<std ::endl;
i=d*i;
std :: cout <<"Multiplication of I & D is :- " <<i
<<std:: endl;
}

bool accept()
{
int tries=1;
while(tries < 4)
{
std :: cout <<"Do you want to proceed (y or n)? \n";
char answer =0;
std :: cin >> answer;
switch(answer)
{
case 'y':
sum_function();
case 'n':
return false;
default :
std :: cout << "Sorry I dont understand that \n";
tries +=1;
}

}
std :: cout <<"I will take for that a no.\n";
return false;
}
 
E

Eric Pruneau

arunix said:
hello all
i am here again. i write this code
how i can be better it?
Thannks in advance..


#include<iostream>
void sum_function();
bool accept();
int main()
{
accept();
// sum_function();
return 0;
}

void sum_function()
{
double d=2.2;
int i=7;
d= d+i;
std :: cout <<"The Sum of D & I is :- " << d
<<std ::endl;
i=d*i;
std :: cout <<"Multiplication of I & D is :- " <<i
<<std:: endl;
}

bool accept()
{
int tries=1;
while(tries < 4)
{
std :: cout <<"Do you want to proceed (y or n)? \n";
char answer =0;
std :: cin >> answer;
switch(answer)
{
Check for upper case Y and N
 
A

arunix

[snip]
Check for upper case Y and N

ok it will be like that

case 'Y':
sum_function();
break;
case 'y':
sum_function();
break;
case 'n':
return false;
break;
case 'N':
return false;
break;
 
I

Ian Collins

arunix said:
[snip]
Check for upper case Y and N

ok it will be like that

case 'Y':
sum_function();
break;
case 'y':
sum_function();
break;
case 'n':
return false;
break;
case 'N':
return false;
break;

you can avoid the duplication:

case 'Y':
case 'y':
sum_function();
break;
case 'N':
case 'n':
return false;
break;

or if you don't care about the case,

switch( toupper(answer) ) {...}
 
A

arunix

remove unneeded line
ok
The message is misleading - this is the result of multiplication of the
sum of original d and i, with i, and truncated to int.

ok
std :: cout <<"Multiplication of I & D(after d=d+i) is :- " <<i
<<std:: endl;
Double space - sloppy typing.

std :: cout <<"Do you want to proceed (y or n)? \n";

break; is probably needed here.

case 'Y':
sum_function();
break;
case 'y':
sum_function();
break;
case 'n':
return false;
break;
case 'N':
return false;
break;
default :
std :: cout << "Sorry I dont understand that \n";
break;
       std :: cout << "Sorry I dont understand that \n";
I think a comma and an apostroph are missing here.
std :: cout << "Sorry I don't understand that. \n";

You are always returning the same value (false) from this function, so
there is no point for the function to return bool in the first place.

because it shows this
"control reaches end of non-void function."
 
I

Ian Collins

[please don't snip attributions, it's rude!]
Its Good But if u gave the other letter then 'y' or 'n' then loop
runs infinite and it stops only at 'n'.

Your original code had a default in the switch. My ... was just for
illustration.
 
A

arunix

[please don't snip attributions, it's rude!]
Its Good  But if u gave the other letter then 'y' or 'n' then loop
runs infinite and it stops only at 'n'.

Your original code had a default in the switch.  My ... was just for
illustration.

ok thanks lan....
 
J

Jonathan Lee

its better than to use
if
else if
else
rather than switch

In what way? I _usually_ find switch easier to read if there's
more than one option. The only thing that I don't like about
switch is the indentation. By time you get to the code, you've
indented 3 times.. i.e.,

void myfunc(int a) {
switch(a) {
case 0:
// do something
break;
case 1:
// etc.
default:
break;
}
}

Doesn't look so bad here, but with 8 character tabs its a
nightmare.

Performance wise, though, it depends a lot on the actual code.

Apparently (on some/most compilers), switch can be optimized
into a jump table. So if you've got a situation when you're
switching on 0, 1, 2, 3,... then the switch will be fast.

On the other hand, if one case occurs a lot and the others
don't, it might be better to do if/elif/else.

Of course, profiling is the only way to know.

--Jonathan
 
J

James Kanze

In what way? I _usually_ find switch easier to read if there's
more than one option.

Or sometimes if there's only one. I use the if for boolean
conditions: is something true, and the switch when I'm logically
thinking in terms of cases: the user entered 'y', the user
entered something else, for example. (Obviously, it's not a
black and white choice. It's conditionned by your point of
view, which isn't an objective fact.)
The only thing that I don't like about switch is the
indentation. By time you get to the code, you've indented 3
times.. i.e.,
void myfunc(int a) {
switch(a) {
case 0:
// do something
break;
case 1:
// etc.
default:
break;
}
}
Doesn't look so bad here, but with 8 character tabs its a
nightmare.

First, since the case's are keywords controlling flow, at the
same level as the switch, you can (and probably should) indent
them at the same level as the switch. (But standards vary: it's
also reasonable to indent them a half a level. The important
part is that the controlled code be indented one level.) Second,
you really shouldn't be using hard tabs: tell your editor to
insert spaces instead of tabs. And set the indent width to
something less than 8. 4 seems about right, but there's
doubtlessly some margin for personal choice here as well.

The one bad thing about switch is that the case doesn't open a
new scope, so you end up having to use additional braces in each
case, i.e.:

switch ( a ) {
case x:
{
// do something here which needs a local
// variable...
}
break;
// ...
}

In practice, though, this turns out to not be much of a problem.
Just about anything complicated to need a local variable should
probably be put in a separate function anyway.

The other weakness of the switch is that the cases all have to
be integral constant expressions. If you need something like:

switch (inputString) {
case "Yes": case "yes":
// do something...
break;

case "No" : case "no":
// do something else...
break;
}

You'll have to use the if ... else if chain.
Performance wise, though, it depends a lot on the actual code.

More on the compiler.
Apparently (on some/most compilers), switch can be optimized
into a jump table. So if you've got a situation when you're
switching on 0, 1, 2, 3,... then the switch will be fast.

First, I've never heard of a compiler which doesn't do this.
The only thing is that the compiler will only do it if the cases
are "dense". Switch over {0, 1, 2, 3, 4}, and you'll certainly
get the jump table. Switch over {'A', 'a', 'x', 'X', '0'}, and
you'll probably get a hard coded binary decision tree (which is
still faster than any direct recoding of the if ... else if...
pattern).

In theory, at least, the compiler could probably do the same
thing with if ... else if ... It would require more work for
the compiler to detect the case, and I don't think many do.

In practice, of course, the difference probably won't be
measurable.
On the other hand, if one case occurs a lot and the others
don't, it might be better to do if/elif/else.

I've heard about at least one compiler which would do this for
you. Now that most compilers can optimize using profiling
output, I would expect such an obvious optimization to become
frequent. (It still won't necessarily outperform the jump
table, but for sparce switches, starting the binary search at
the most frequent case, rather than in the middle, could be a
win if the frequencies are squewed.)
 
J

James Kanze

[...]
remove unneeded line

The line is needed: the function is defined to return an int, so
the last line of the function should be a return statement with
an int. I know that in this particular case, the compiler will
do that for you, but it's bad practice to count on it: either
the name of the function will change (because the program is
integrated into a larger program), or the return value will
change. (Programs which fail never just return 0. So any
program that can fail in any way---and that includes any program
with output or input---must be capable of returning something
other than 0.)
 
J

James Kanze

[...]
case 'Y':
sum_function();
break;
case 'y':
sum_function();
break;
case 'n':
return false;
break;
case 'N':
return false;
break;
default :
std :: cout << "Sorry I dont understand that \n";
break;
std :: cout << "Sorry I don't understand that. \n";
because it shows this
"control reaches end of non-void function."

As written, the function always returns false, regardless. So
there's no point in it returning anything; it should be declared
void. What you probably meant for it to do is something like:

enum Results { yes, no, error };

Results
getAction()
{
int retryCount = 0;
Results result = error;
while ( retryCount < maxRetries && result == error ) {
switch ( getChar() ) {
case 'y':
case 'Y':
result = yes;
break;

case 'n':
case 'N':
result = no;
break;
}
++ retryCount;
}
return result;
}

Returning a constant is usually a sign of an error (unless
you're overriding a virtual function).
 
J

Jonathan Lee

First, since the case's are keywords controlling flow, at the
same level as the switch, you can (and probably should) indent
them at the same level as the switch.

I've tried this and I just can't get used to it. I guess it's
my own damn fault. Same for the tabs.
The one bad thing about switch is that the case doesn't open a
new scope, so you end up having to use additional braces in each
case,

I've had this situation for temps. Although case doesn't open a
new scope, switch does. I just stick the declarations of the temps
there, before the first case. Not ideal, but... *shrugs*
More on the compiler.

I mean if you have branches which have equal probability, leave
it up to the compiler. But if you know branches occur very
unevenly, use the if...else. Consider this code I have for getting
a little endian 32-bit value from a block of memory. It goes
roughly like this (making assumptions about "byte" size, etc.):

unsigned long memblock::le32(size_t i) {
if (i + 3 < block_size) {
// load all four bytes
} else { // near end of block
if (i + 2) < block_size) {
// only one bytes
} else if (i + 1 < block_size) {
// only two bytes
} else { }
}
}

The first branch happens almost exclusively. This is perhaps
an exaggerated example, but I think you can see the point:
ensure that the most frequently executed code is reached in
the shortest amount of time.

Of course, as always, profile to be sure. Or, as you mention
below, compilers can take profiler output. I get mixed results
with this. I get known results with the above.
First, I've never heard of a compiler which doesn't do this.

Some version of GCC newer than 4.2 didn't do this for me. I had a
loop counting from 0 to 80 (I think... it was for the serpent
block cipher), and a switch on the the loop modulo 16. Even with
high optimization turned on it was still (30% or so) slower than
nested ifs:

if (x < 8) {
if (x < 4) {
if (x < 2) { ... } else { ... }
} else {
if (x < 6) { ... } else { ... }
}
} else { /* similarly */ }

I didn't check the disassembly, but I would think if there was
a jump table, that would be faster than three comparisons.
But then maybe on the CPU I was using the branch prediction was
great, and jumping via a table thrashed the instruction cache.

--Jonathan
 
S

Stefan Ram

Jonathan Lee said:
But then maybe on the CPU I was using the branch prediction was
great, and jumping via a table thrashed the instruction cache.

With static branch prediction, which label would usually be
predicted by a modern CPU? In

if( expression )
{ A: ... }
B: ...

and

if( expression )
{ A: ... }
else
{ B: ... }

?
 
J

Jonathan Lee

  With static branch prediction, which label would usually be
  predicted by a modern CPU? In

"A" in both cases since it's always cheaper. "A" will already be
in the pipeline.

But modern CPUs will use a history to do branch prediction, and
the nested ifs take advantage of that. They either alternate
(taken or not) or consecutively do the same thing
(taken taken not-taken not-taken). I don't know if the CPU can
do that kind of prediction on a jump table.

I'm not really sure what you're getting at, though. If you mean
that I shouldn't rely on dynamic branch prediction then, OK. I
agree. But I happen to know my platform has it. I profiled both
versions and one was the clear winner.

--Jonathan
 
J

Jonathan Lee

Slight error. I should have been switching on 0..7 not 0..15

Not this:
  if (x < 8) {
    if (x < 4) {
      if (x < 2) { ... } else { ... }
    } else {
      if (x < 6) { ... } else { ... }
    }
  } else { /* similarly */ }

But this:

if (x < 4) {
if (x < 2) {
if (x < 1) { ... } else { ... }
} else {
if (x < 3) { ... } else { ... }
}
} else { /* similarly */ }

Doesn't make much of a difference, though. Just that
each "case" has its own body. (Instead of case 0 and
case 1 being the same, etc.)

--Jonathan
 
S

Stefan Ram

Jonathan Lee said:
I'm not really sure what you're getting at, though.

I was getting at nothing - I just wanted to
learn what I asked about.

It is not obvious to me, what is in the cache,
because a compiler might compile

if( expression )
{ A: ... }
else
{ B: ... }

as

if( !expression )
{ B: ... }
else
{ A: ... }

would be compiled by another compiler, or might compile

if( expression )
{ A: ... }
B: ...

as another compiler would compile

if( !expression )goto over;
A: ...
over: B: ...

So, it is more difficult to make assertions about
branch prediction from C++ source code than from
assembler code.
 
J

Jonathan Lee

  I was getting at nothing - I just wanted to
  learn what I asked about.

Sorry, I misunderstood.
  It is not obvious to me, what is in the cache,
  because a compiler might compile

<snip examples>

True, it's possible. Though, I think static branch prediction
would favour compiling the code in the given order. That's
pure speculation of course; I just don't see why any
compiler would do otherwise. If in doubt, check the docs.
if( expression )
{ A: ... }
B: ...

  as another compiler would compile

if( !expression )goto over;
A: ...
over: B: ...

On (at least) x86 those are the same thing. The different
thing would be

if (expression) goto A;
B: // common stuff
...
goto C:
A: // do the "if" stuff
...
goto B; // resume flow
C: // anything which might come after B

Which is just plain silly.

--Jonathan
 
J

James Kanze

On Dec 20, 7:11 am, James Kanze <[email protected]> wrote:

[...]
I've had this situation for temps. Although case doesn't open
a new scope, switch does. I just stick the declarations of the
temps there, before the first case. Not ideal, but... *shrugs*

That's illegal, and shouldn't compile. In general, the
semantics of a switch are those of a goto, and jumping over a
definition with an initializer is illega.
I mean if you have branches which have equal probability,
leave it up to the compiler. But if you know branches occur
very unevenly, use the if...else.

Why? The compiler is far more capable than you of counting the
exact frequencies.
Consider this code I have for getting a little endian 32-bit
value from a block of memory. It goes roughly like this
(making assumptions about "byte" size, etc.):
unsigned long memblock::le32(size_t i) {
if (i + 3 < block_size) {
// load all four bytes
} else { // near end of block
if (i + 2) < block_size) {
// only one bytes
} else if (i + 1 < block_size) {
// only two bytes
} else { }
}
}
The first branch happens almost exclusively. This is perhaps
an exaggerated example, but I think you can see the point:
ensure that the most frequently executed code is reached in
the shortest amount of time.

Yes. That's what a good compiler does. It ensures that the
most frequently executed branches are the most optimized. (Your
example reminds me of Duff's device. Or how to abuse switch
statments.)
Of course, as always, profile to be sure. Or, as you mention
below, compilers can take profiler output.

Are there any left that don't? A lot of the time, you don't
bother, because the code is fast enough as is, but when it
matters, the compiler does a better job of it than you do.
I get mixed results with this. I get known results with the
above.

Do you?
Some version of GCC newer than 4.2 didn't do this for me. I
had a loop counting from 0 to 80 (I think... it was for the
serpent block cipher), and a switch on the the loop modulo 16.
Even with high optimization turned on it was still (30% or so)
slower than nested ifs:
if (x < 8) {
if (x < 4) {
if (x < 2) { ... } else { ... }
} else {
if (x < 6) { ... } else { ... }
}
} else { /* similarly */ }
I didn't check the disassembly, but I would think if there was
a jump table, that would be faster than three comparisons.
But then maybe on the CPU I was using the branch prediction
was great, and jumping via a table thrashed the instruction
cache.

It would be necessary to examine the exact code you were
comparing, and then look at the generated code to be sure. For
something like the above, I'd imagine that something like:

switch ( x % 8 )
{
// ...
}

would be fastest.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,483
Members
44,902
Latest member
Elena68X5

Latest Threads

Top