From: tonydee on
On May 6, 4:42 pm, Nathan <nathancba...(a)gmail.com> wrote:
> On May 6, 2:11 am, tonydee <tony_in_da...(a)yahoo.co.uk> wrote:
> >   while ( data[xInd][yInd][zInd] != value && result != 0)
> >   {
> >       if {++zInd == data[xInd][yInd].size()) {
> >           zInd = 0;
> >           if (++yInd == data[xInd].size()) {
> >               yInd = 0;
> >               if (++xInd == data.size())
> >                   result = 0;
> >           }
> >       }
> >   ...
>
> Isn't <something>.size() a method or function call?

Yes.

> We'd also want to
> eliminate that needless activity from the loop by defining "zMax =
> data[xInd][yInd].size()", and etc., before the loop.

Possibly - some people do exactly that. Still, using trivial inline
member function calls to access data member is such an important part
of encapsulation in OO programming that all modern C++ optimisers
definitely produce equally efficient code despite the repeated calls
to std::vector<>::size(). That's true for most containers, but not
all. The concern has more validity if the container type is unknown
(e.g. a template parameter), or there's concern it might change during
maintenance. Counter-balancing that is the simple, idiomatic usage
and concision as presented. Delocalising the arguments in the
comparisons by putting the values in local variables on other lines,
which by default remains in scope much longer than you're likely to
need it (possibly with a value that is invalidated by subsequent
code), tends to have a net detrimental effect on code clarity and
correctness, and for no performance benefit in this case.

> Just because C++ gifts you with 'high-level' abstractions, that
> doesn't confer an automatic excuse to create inefficient,
> unmaintainable code.

A good C++ programmer understands what the optimiser can be expected
to do, and creates clean, concise code when possible, and more
explicitly assists the compiler/optimiser when actually beneficial.

Cheers,
Tony
From: Nathan Baker on
"Keith Thompson" <kst-u(a)mib.org> wrote in message
news:ln4oilxyd9.fsf(a)nuthaus.mib.org...
>
> A style point: comparisons to true and false are almost always
> superfluous. Rather than
> if (( xInd < data.size() ) != true )
> just write
> if ( xInd <= data.size() )
>

A true point. But I guess, for comparison sake, it is superfluous. :)

> The original problem is to traverse a 3-dimensional array. A triple
> nested loop is the most obvious way to do that. There might be
> some advantages in converting it to a single loop, but clarity
> isn't one of them, at least in this case.
>

I guess, the dragon that I am pointing at, is that CircuitCity does not yet
sell 3-dinensional RAM. So, why not just do...

,---
xyzMax = data[xInd][yInd].size() + data[xInd].size() + data.size();
result = 0;
i = -1;
while ( i < xyzMax )
{
++i;
if ( data[i] = value );
{
result = &data[i];
i = xyzMax;
}
}
return result;
`---

....a linear solution to a linear problem???

Nathan.


From: Ike Naar on
In article <ln4oilxyd9.fsf(a)nuthaus.mib.org>,
Keith Thompson <kst-u(a)mib.org> wrote:
>A style point: comparisons to true and false are almost always
>superfluous. Rather than
> if (( xInd < data.size() ) != true )
>just write
> if ( xInd <= data.size() )

It's not the same thing (``<='' should be ``>='').
>
>As for the overall structure of your proposed replacement, you've
>replaced three nested loops with one loop and three if statements.
>Furthermore, the original version only tests zInd on most iterations;
>your version tests zInd, yInd, and xInd on every iteration.
>
>The original problem is to traverse a 3-dimensional array. A triple
>nested loop is the most obvious way to do that. There might be
>some advantages in converting it to a single loop, but clarity
>isn't one of them, at least in this case.
>
>--
>Keith Thompson (The_Other_Keith) kst-u(a)mib.org <http://www.ghoti.net/~kst>
>Nokia
>"We must do something. This is something. Therefore, we must do this."
> -- Antony Jay and Jonathan Lynn, "Yes Minister"


From: Keith Thompson on
ike(a)localhost.claranet.nl (Ike Naar) writes:
> In article <ln4oilxyd9.fsf(a)nuthaus.mib.org>,
> Keith Thompson <kst-u(a)mib.org> wrote:
>>A style point: comparisons to true and false are almost always
>>superfluous. Rather than
>> if (( xInd < data.size() ) != true )
>>just write
>> if ( xInd <= data.size() )
>
> It's not the same thing (``<='' should be ``>='').

Argh!

It was correct in my head.

--
Keith Thompson (The_Other_Keith) kst-u(a)mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
From: Juha Nieminen on
In comp.lang.c++ Nathan <nathancbaker(a)gmail.com> wrote:
> On Apr 28, 1:16�am, Juha Nieminen <nos...(a)thanks.invalid> wrote:
>> Value_t* MyClass::findValue(const Value_t& value)
>> {
>> for(size_t xInd = 0; xInd < data.size(); ++xInd)
>> for(size_t yInd = 0; yInd < data[xInd].size(); ++yInd)
>> for(size_t zInd = 0; zInd < data[xInd][yInd].size(); ++zInd)
>> {
>> if(data[xInd][yInd][zInd] == value)
>> return &data[xInd][yInd][zInd];
>> }
>>
>> return 0;
>> }
>
> Hi Juha,
>
> I think that you either copied this from a poorly-written beginner C++
> book or you failed to understand what the author was attempting to
> demonstrate with that kind of code and that you did not 'catch on'
> that it is not (in any way) demonstrative of how one walks a series of
> sequential cells in the real world.

I don't know if you are being sarcastic, patronizing, or honestly don't
know what you are talking about, but I'll let it pass.

> Why do you insist on writing control structures that serve little
> actual purpose? For instance, it should be obvious that you only
> require ONE loop and a few conditionals to achieve your desired (or
> assumed) goal. E.G...
>
> ,---
> result = value
> xInd = 0
> yInd = 0
> zInd = 0
> while ( data[xInd][yInd][zInd] != value && result != 0)
> {
> ++zInd;
> if (( zInd < data[xInd][yInd].size() ) != true ) { zInd = 0; +
> +yInd};
> if (( yInd < data[xInd].size() ) != true ) { yInd = 0; ++xInd};
> if (( xInd < data.size() ) != true ) { result = 0 };
> }
> return result
> `---

The goal was to make the function simpler, not more complicated.
Your version (even if it's fixed to actually do the same thing, and even
after removing the superfluous conditionals) is significantly more
complicated for many reasons. Most importantly, it uses a strange idiom
which the vast majority of programmers are not familiar with and don't
use (and why would they?) It takes significantly more time to understand
what your code is doing, as well as to verify that it's doing it correctly.
My original version with three nested loops is a lot easier to understand,
and a lot easier to verify that it works correctly.

And what for? It's not more efficient, shorter or does it do anything
that the original wouldn't do. Moreover, it's very possible that in some
similar circumstances a compile could be able to perform better optimizations
on the original nested loop version than yours (eg. by applying loop
unrolling and other optimization techniques).

> Of course, this is a rather useless (maybe even retarded) function to
> begin with, because it only tells you IF the 'value' is located
> "somewhere" within that array -- it gives you absolutely no indication
> of "where" in that array you might be able to access the item which
> matches your search criteria.

Actually the function returns a pointer to the found element, or null
if the element is not found.