«

I found one of that problems all of us have solve when they begin
programming: given 3 numbers print the greater and the lesser one of
the set.

I was trying to remember the if-then-else structure when something hit
me and I wrote:

// Function returns the greater or lesser numbers of two given numbers,

// specified by a flag: 1 for greater (default), 0 for lesser

int greater_lesser(int a, int b, bool flag=1){

int toBeReturned;

if(flag)
a > b ? toBeReturned = a : toBeReturned = b;
else
a > b ? toBeReturned = b : toBeReturned = a;

return toBeReturned;
}

int main(){

int x, y, z;
cin >> x >> y >> z;
cout << "Greater : " << greater_lesser(greater_lesser(x,y),z) << endl
<< "Lesser : " << greater_lesser(greater_lesser(x,y,0),z,0) << endl;

return 0;
}

The code works all right, but it seems odd, as if it can be improved...

Any thoughts? Thanks!

Comments on your code inline:

"Gaijinco" <> wrote in message
news: oups.com...
>I found one of that problems all of us have solve when they begin
> programming: given 3 numbers print the greater and the lesser one of
> the set.
>
> I was trying to remember the if-then-else structure when something hit
> me and I wrote:
>
> // Function returns the greater or lesser numbers of two given numbers,
>
> // specified by a flag: 1 for greater (default), 0 for lesser
>
> int greater_lesser(int a, int b, bool flag=1){

bool flag = true


> int toBeReturned;

Don't need this.

>
> if(flag)
> a > b ? toBeReturned = a : toBeReturned = b;

return a > b ? a : b;

> else
> a > b ? toBeReturned = b : toBeReturned = a;

return a > b ? b : a;

>
> return toBeReturned;

Don't need this

> }
>
> int main(){
>
> int x, y, z;
> cin >> x >> y >> z;
> cout << "Greater : " << greater_lesser(greater_lesser(x,y),z) << endl
> << "Lesser : " << greater_lesser(greater_lesser(x,y,0),z,0) << endl;
>
> return 0;
> }
>
> The code works all right, but it seems odd, as if it can be improved...
>
> Any thoughts? Thanks!

Now, this is not the way I would do it anyway. If I wanted a function to
give me the greater, or lesser of numbers I would allow the number of
elements to be arbitrary. Meaning passing in a reference to a vector and
returning the greater one. There might even already be some STL function
that does that, I don't know enough about all the STL functions to say.

There are many ways to do that, but I'll let others comment on STL methods
to do it before I post anything else cause I'm fairly sure it's simple with
some container iteratation.

Gaijinco wrote:

> I found one of that problems all of us have solve when they begin
> programming: given 3 numbers print the greater and the lesser one of
> the set.
>
> I was trying to remember the if-then-else structure when something hit
> me and I wrote:
>
> // Function returns the greater or lesser numbers of two given numbers,
>
> // specified by a flag: 1 for greater (default), 0 for lesser
>
> int greater_lesser(int a, int b, bool flag=1){
>
> int toBeReturned;
>
> if(flag)
> a > b ? toBeReturned = a : toBeReturned = b;
> else
> a > b ? toBeReturned = b : toBeReturned = a;
>
> return toBeReturned;
> }
>
> int main(){
>
> int x, y, z;
> cin >> x >> y >> z;
> cout << "Greater : " << greater_lesser(greater_lesser(x,y),z) << endl
> << "Lesser : " << greater_lesser(greater_lesser(x,y,0),z,0) << endl;
>
> return 0;
> }
>
> The code works all right, but it seems odd, as if it can be improved...

Why reinvent the wheel?

#include <algorithm>
#include <iostream>

int main ( void ) {
int x, y, z;
std::cin >> x >> y >> z;
std::cout << "max: " << std::max( std::max( x, y ), z ) << '\n'
<< "min: " << std::min( std::min( x, y ), z ) << '\n';
}


Best

Kai-Uwe Bux

"Gaijinco" <> wrote in message
news: oups.com...
> I found one of that problems all of us have solve when they begin
> programming: given 3 numbers print the greater and the lesser one of
> the set.
>
> I was trying to remember the if-then-else structure when something hit
> me and I wrote:
>
> // Function returns the greater or lesser numbers of two given numbers,
>
> // specified by a flag: 1 for greater (default), 0 for lesser
>
> int greater_lesser(int a, int b, bool flag=1){
>
> int toBeReturned;
>
> if(flag)
> a > b ? toBeReturned = a : toBeReturned = b;
> else
> a > b ? toBeReturned = b : toBeReturned = a;
>
> return toBeReturned;
> }
>
> int main(){
>
> int x, y, z;
> cin >> x >> y >> z;
> cout << "Greater : " << greater_lesser(greater_lesser(x,y),z) << endl
> << "Lesser : " << greater_lesser(greater_lesser(x,y,0),z,0) << endl;
>
> return 0;
> }
>
> The code works all right, but it seems odd, as if it can be improved...
>
> Any thoughts? Thanks!
>

No, this is horrible!

Flags which control the behavior of a function in such a radical way are
often
confusing and make code difficult to understand, especially something as
simple
as this. Perhaps in a more complex example where a basic algorithm can be
adapted
to perform different variations, flags might be more appropriate, but I
would generally
make that function private and provide simple wrappers of the basic function
to be more
meaningful to a user.

Flags have a habit of multiplying, so we often see two or three flag
parameters in code which
affect the behavior of the function making code harder to understand.


Surely the following is easier to understand ?

#include <iostream>


int lesser( int a, int b)
{
return a < b ? a : b;
}
int greater( int a, int b)
{
return a > b ? a : b;
}
int main()
{
int x, y, z;
std::cin >> x >> y >> z;
std::cout << "Greater : " << greater ( greater (x,y),z) << "\n"
<< "Lesser : " << lesser( lesser(x,y),z) << "\n";

return 0;
}

Gaijinco wrote:
>
> int greater_lesser(int a, int b, bool flag=1){
>
> int toBeReturned;
>
> if(flag)
> a > b ? toBeReturned = a : toBeReturned = b;
> else
> a > b ? toBeReturned = b : toBeReturned = a;
>
> return toBeReturned;
> }
>
> Any thoughts? Thanks!

9 times out of 10, if you have a function of the form:

func(bool flag)
{
// no code before if

if (flag)
...
else
...

// no code after if/else
}

such that there is ***no common code*** before or after the if/else,
then you really have 2 completely separate functions, right?

P.S. When I say '9 times out of 10', in this case I really mean 10
times out of 10.

- Tony

Gaijinco wrote:
> I found one of that problems all of us have solve when they begin
> programming: given 3 numbers print the greater and the lesser one of
> the set.
>
> I was trying to remember the if-then-else structure when something hit
> me and I wrote:
>
> // Function returns the greater or lesser numbers of two given numbers,
> // specified by a flag: 1 for greater (default), 0 for lesser

This is totally bogus, regardless of the implementation.

On Sat, 04 Feb 2006 20:02:46 -0800, Gaijinco wrote:

> I found one of that problems all of us have solve when they begin
> programming
<snip>
> // specified by a flag: 1 for greater (default), 0 for lesser
>
> int greater_lesser(int a, int b, bool flag=1){

Sticking to general matters, in those situations where you need a boolean
flag (this not being one of them) possibly the very worst name for it is
"flag"! Choose a name that identifies what it is "flagging" when true:

bool candidate_found = false;
bool number_has_dot = strchr(number, '.') != NULL;

and so on.

--
Ben.

It just seemed that a function

int max(int a,int b){
return a > b ? a : b;
}

and another one like

int min(int a,int b){
return a > b ? b : a;
}

Were so similar that they could be rewritten as a single function so
the use of a flag seemed valid. I recognize that a flag named flag is
really dumb.

But if something like

int max_min(int a, int b, bool max=true){

if(max)
return a > b ? a : b;
else
return a > b ? b : a;
}

is a misuse of a flag, then what isn't?

Ben Bacarisse wrote:
> Sticking to general matters, in those situations where you need a boolean
> flag (this not being one of them) possibly the very worst name for it is
> "flag"! Choose a name that identifies what it is "flagging" when true:
>
> bool candidate_found = false;
> bool number_has_dot = strchr(number, '.') != NULL;

I find an exception, in practice, for boolean mutators:

private:
bool happy;
public:
void setHappy(bool flag);

Given that the function name and parameter type contain all the
relevant information, what's the point in trying to express something
additional in the name of the parameter? What would you want to call
it? The best alternative I know is along the lines of:

private:
bool happy_; // Sutter-style trailing underscores for private
member data
public:
public setHappy(bool happy);

But I really don't like keeping the same information in two places like
that. "flag" is perfectly descriptive, when there just isn't enough
semantic content that anything else useful could be conveyed.

Luke

Luke Meyers wrote:
> Ben Bacarisse wrote:
> > Sticking to general matters, in those situations where you need a boolean
> > flag (this not being one of them) possibly the very worst name for it is
> > "flag"! Choose a name that identifies what it is "flagging" when true:
> >
> > bool candidate_found = false;
> > bool number_has_dot = strchr(number, '.') != NULL;
>
> I find an exception, in practice, for boolean mutators:
>
> private:
> bool happy;
> public:
> void setHappy(bool flag);
>
> Given that the function name and parameter type contain all the
> relevant information, what's the point in trying to express something
> additional in the name of the parameter? What would you want to call
> it?

What about setHappy(bool turnOn)

Your's makes the assumption that happy is a bool. What if it is a
string? Maybe setHappy(bool) is something like this:

void setHappy(bool on)
{
happy = on ? "I'm happy":"I'm ****y";
}

Granted, one could see the information in the class definition but you
never know; maybe you'll want to change that later. Might be nice to
call the 'flag' variable by what effect it might have.

On the other hand, IMHO the 'setHappy()' function shouldn't exist.
There should be two of them:

void makeHappy();
void makeUnhappy();

Now there is no knowledge of, nor reference to, any internal data
whatsoever...these functions could have any effect or combination of
effects. It also gets rid of an if since in the original your code
ends up being something like (assuming set happy got more complex, as
it naturally does):

if (?) flag = true;
else flag = false;

if (flag)
...
else
...

Finally, if you have a variable that makes sense as a "flag" then you
have an area that could use improvement in design. What effect does
the "happy" flag have? One would assume that it is later used to base
decisions on in other member variables and possibly client classes;
branches. It would be better to use a more OO design - possibly the
state pattern but others might be better depending on the effects
'happy' has.