divide all elements in a vector by a number

[Kai-Uwe Bux]

Well, the one David harman posted was what I was thinking about.

That would be this:

std::transform(v.begin(), v.end(), v.begin(),

I grant
that it would have undefined behavior if the maximum element is 0
(however, isn't 0/0 well defined?)

a) 0/0 is not well-defined, neither in math nor in C++.

b) The maximum in the vector can be 0 while other elements are not: they
could be all negative.

but it would work fine if the array is empty.

Nope. The posted version seems to be missing the dereferencing of
max_element, which returns an iterator. If dereferencing is done, it will
have UB for empty vectors:

std::transform(v.begin(), v.end(), v.begin(),
std::bind2nd(std::divides<double>,
* std::max_element(v.begin(), v.end()));

The point is that the dereferencing is done _before_ std::transform has a
chance to tell that nothing needs to be done.

I would probably preface it with an assert to make sure that such isn't
the case.

Best

Kai-Uwe Bux

 

[James Kanze]

Daniel T. wrote:
I was wondering if I could divide all elements of a
vector by the max element of that vector.

[...]

Although there are mulitple ways of doing this in one line
as an academic exercise, I think performance and
readiability wise, a 2 liner is better for practical
purposes. I don't doubt your intention but let's not send
out the wrong signal.

I think by "one line", he really meant one C++ statement.
I've never managed to get even the simplest call to
std::transform on a single line, while still keeping a
reasonable line length. (If humans are to read the code,
something about 60 characters, not counting indentation, is
about the limit. You can exceed it occassionally, but 80
characters is a good absolute limit.)
David Harmon's suggestion is only one C++ statement, is very
readable, and probably results in the best performance
you're likely to get.

I must be missing something here. The one-command versions
(and also the 2-command ones) that I can see are either not
readable or not correct (at least they have undefined behavior
when the vector is empty or when the maximum element is 0).

Touche. You can handle the empty array case in a single
statement, using ?:, of course, but an if would be more natural
(since there's nothing to do in once case, and (void)0 is not
really very "natural"). For the case where the maximum element
is 0, you really have to define what is wanted---maybe a simple
pre-condition (all elements > 0.0) is all that is wanted. But
you're right that unless the code has considered the case, it's
not correct.

 

[James Kanze]

That would be this:

std::transform(v.begin(), v.end(), v.begin(),
std::bind2nd(std::divides<double>,
std::max_element(v.begin(), v.end()));

He later posted a correction, adding a dereference operator in
front of std::max_element. (This version has undefined
behavior, since the return value of std::max_element doesn't
meet the constraints on an argument to std::divides<double>. In
practice, I doubt that it will ever compile.)

[...]

Nope. The posted version seems to be missing the dereferencing of
max_element, which returns an iterator. If dereferencing is done, it will
have UB for empty vectors:

std::transform(v.begin(), v.end(), v.begin(),
std::bind2nd(std::divides<double>,
* std::max_element(v.begin(), v.end()));

The point is that the dereferencing is done _before_
std::transform has a chance to tell that nothing needs to be
done.

It's easy enough to fix for the empty array case, so it would be
silly not to. Something like:

if ( ! v.empty() ) {
std::transform(v.begin(), v.end(), v.begin(),
std::bind2nd(std::divides<double>,
* std::max_element(v.begin(), v.end()));
}

or if you really want just a single statement:

v.empty()
? (void)0
: (void)std::transform(
v.begin(), v.end(), v.begin(),
std::bind2nd(std::divides<double>,
* std::max_element(v.begin(), v.end()));

or
v.empty()
? v.end()
: std::transform(
v.begin(), v.end(), v.begin(),
std::bind2nd(std::divides<double>,
* std::max_element(v.begin(), v.end()));

(Despite the higher statement count, I'd go with the first,
unless I needed the return value provided by the last---which
would be pretty surprising.)

Also, you could replace the "*std::max_element(v.begin(),
v.end())" subexpression with std::max( 1, *std::max_element...
to avoid undefined behavior, but I rather question the
usefulness of the resulting semantics. Something like:

*std::max_element( ... ) == 0
? throw someDocumentedError
: *std::max_element( ... )

would do the trick, but it seems overdoing the "avoid premature
optimization" rule, even to me, since it iterates over the
entire vector twice, when once would do. (Theoretically, I
guess, a compiler could optimize out the second iteration, but
it would take some doing, and I rather doubt that any do. And
the implementation cannot even use a compiler specific
annotation to tell the compiler that max_element is a pure
function, since it's a template, and if any of the actions on
the iterators have side effects, it's not.)

 

[Kai-Uwe Bux]

That would be this:

std::transform(v.begin(), v.end(), v.begin(),
std::bind2nd(std::divides<double>,
std::max_element(v.begin(), v.end()));

He later posted a correction, adding a dereference operator in
front of std::max_element. (This version has undefined
behavior, since the return value of std::max_element doesn't
meet the constraints on an argument to std::divides<double>. In
practice, I doubt that it will ever compile.)

[...]

Nope. The posted version seems to be missing the dereferencing of
max_element, which returns an iterator. If dereferencing is done, it will
have UB for empty vectors:

std::transform(v.begin(), v.end(), v.begin(),
std::bind2nd(std::divides<double>,
* std::max_element(v.begin(), v.end()));

The point is that the dereferencing is done _before_
std::transform has a chance to tell that nothing needs to be
done.

It's easy enough to fix for the empty array case, so it would be
silly not to. Something like:

if ( ! v.empty() ) {
std::transform(v.begin(), v.end(), v.begin(),
std::bind2nd(std::divides<double>,
* std::max_element(v.begin(), v.end()));
}

or if you really want just a single statement:

v.empty()
? (void)0
: (void)std::transform(
v.begin(), v.end(), v.begin(),
std::bind2nd(std::divides<double>,
* std::max_element(v.begin(), v.end()));

or
v.empty()
? v.end()
: std::transform(
v.begin(), v.end(), v.begin(),
std::bind2nd(std::divides<double>,
* std::max_element(v.begin(), v.end()));

(Despite the higher statement count, I'd go with the first,
unless I needed the return value provided by the last---which
would be pretty surprising.)

I also would go with the first version.

Also, you could replace the "*std::max_element(v.begin(),
v.end())" subexpression with std::max( 1, *std::max_element...
to avoid undefined behavior, but I rather question the
usefulness of the resulting semantics.

The semantics would not be equivalent in the cases where
*std::max_element(...) < 0.

Something like:

*std::max_element( ... ) == 0
? throw someDocumentedError
: *std::max_element( ... )

would do the trick, but it seems overdoing the "avoid premature
optimization" rule, even to me, since it iterates over the
entire vector twice, when once would do. (Theoretically, I
guess, a compiler could optimize out the second iteration, but
it would take some doing, and I rather doubt that any do. And
the implementation cannot even use a compiler specific
annotation to tell the compiler that max_element is a pure
function, since it's a template, and if any of the actions on
the iterators have side effects, it's not.)

So, putting it all together, we have the following impressive one-statement
solution:

v.empty()
? v.end()
: std::transform(
v.begin(), v.end(), v.begin(),
std::bind2nd(std::divides<double>,
*std::max_element( v.begin(), v.end() ) == 0
? throw someDocumentedError
: *std::max_element( v.begin(), v.end() ) ) );


Wow! I would go with something more conventional:

if ( ! v.empty() ) {
double denominator = *std::max_element( v.begin(), v.end() );
if ( denominator == 0 ) {
throw something;
} else {
std::transform( v.begin(), v.end(), v.begin(),
std::bind2nd( std::divides<double>(), denominator ) );
}
}


Best

Kai-Uwe Bux

 

[James Kanze]

[...]

The semantics would not be equivalent in the cases where
*std::max_element(...) < 0.

Yes. It's only valid if you have a pre-condition along the
lines of all elements > 0.

So, putting it all together, we have the following impressive one-statement
solution:

v.empty()
? v.end()
: std::transform(
v.begin(), v.end(), v.begin(),
std::bind2nd(std::divides<double>,
*std::max_element( v.begin(), v.end() ) == 0
? throw someDocumentedError
: *std::max_element( v.begin(), v.end() ) ) );

Wow! I would go with something more conventional:

if ( ! v.empty() ) {
double denominator = *std::max_element( v.begin(), v.end() );
if ( denominator == 0 ) {
throw something;
} else {
std::transform( v.begin(), v.end(), v.begin(),
std::bind2nd( std::divides<double>(), denominator ) );
}
}

Unless, of course, it was the entry into some sort of contest,
along the lines of IOCCC, but for C++.

But that may be just you and me. When I look at lisp code, it
all looks like the less conventional version to me, but lispians
seem to find it quite natural. (But I don't think we're alone.)

 

[terminator]

I see this is also a nice suggestion since my concern for using C++ is
mainly on number crunching.

all the above are based on loops ,so you cantry recursion yourself.

regards,
FM.