From: Daniel T. on 7 May 2010 00:23 In article <7ec1a55b-4a14-46c3-bb65-dd5de1aa370b(a)n15g2000yqf.googlegroups.com>, tonydee <tony_in_da_uk(a)yahoo.co.uk> wrote: > On May 6, 2:36�pm, Nathan <nathancba...(a)gmail.com> wrote: > > On Apr 28, 1:16�am, Juha Nieminen <nos...(a)thanks.invalid> wrote: > > > � Care to show an actual example of your "simpler, cleaner and easier to > > > follow" version of exiting a nested loop by meddling with the loop > > > conditions instead of using 'return'? For example, modify the following > > > code to conform to your specifications: > > > > > 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. > > > > 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 > > `--- > > Juha did ask for something "simpler, cleaner and easier to follow", > which your single loop is not. Perhaps because of that, your code > needlessly compares yInd and xInd to their respective limits on every > iteration, rather than something like... > > 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; > } > } > ... > > More importantly, it can dereference index [0] before checking size(), > so produces undefined behaviour. Anyway, IMHO it's far less clear > (=self-evidently correct & efficient as well as maintainable) than > Juha's code. > > > 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. > > You missed that the original function was returning a pointer to the > matching cell, allowing a change to be made at that location, > considerably more useful than the search-term-else-0-sentinel version > you coded. > > > Please pardon any butchering of C++ syntax in my pseudo snippet above > > -- I totally lack any C++ training. > > Fair enough... no worries. > > >�But we alt.lang.asm folk *do* > > have an inkling of how to actually code our way out of a paper bag... > > I do believe. > > > > Nathan. > > Your central point that a single loop can serve contributes an > interesting alternative, which I'm sure the readers here will > appreciate, if they bother to look past your smug attitude (which is > quite unwarranted given the serious errors in your implementation).... I posted a single loop alternative last week or so. I think it is a much better solution than Juha's original.
From: Daniel T. on 7 May 2010 00:26 Nathan <nathancbaker(a)gmail.com> wrote: > On May 6, 2:11�am, tonydee <tony_in_da...(a)yahoo.co.uk> wrote: > > > > Juha did ask for something "simpler, cleaner and easier to follow", > > which your single loop is not. �Perhaps because of that, your code > > needlessly compares yInd and xInd to their respective limits on every > > iteration, rather than something like... > > > > � 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? We'd also want to > eliminate that needless activity from the loop by defining "zMax = > data[xInd][yInd].size()", and etc., before the loop. > > Just because C++ gifts you with 'high-level' abstractions, that > doesn't confer an automatic excuse to create inefficient, > unmaintainable code. It is precisely the fact that Juha's code doesn't use high level abstractions that is the problem. See my earlier posts on this sub-thread.
From: io_x on 7 May 2010 04:18 "Juha Nieminen" <nospam(a)thanks.invalid> ha scritto nel messaggio news:4be2f44f$0$11839$7b1e8fa0(a)news.nbl.fi... > 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) pheraps i don't understand C or C++; are you sure that "data.size()" return the max index of x? in how i read it, it could return the number of NxNxN table elements or the size in bytes of data object than where "data" came from: is it global? >>> 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; >>> } could be this ok for a mXmXm matrix that has the index in the same range 0..m? Value_t* MyClass::findValue(Value_t& value) {size_t x, y, z, m; m=sizeof(data)/sizeof(data[0][0][0]); m=sqrt3(m); // 3 radix for(x=0; x <m; ++x ) for(y=0; y<m; ++y) for(z=0; z<m; ++z) if(data[x][y][z]==value) return &data[x][y][z]; 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. in the code you write, for me nothing is clear it is not question if "is it easy?", it is question "is it right?" >> 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.
From: Nick Keighley on 7 May 2010 04:17 On 6 May, 08:50, Keith Thompson <ks...(a)mib.org> wrote: > 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 considered submitting a single loop solution as a joke. It never crossed my mind someone would seriusly propose it!
From: Juha Nieminen on 7 May 2010 06:34
In comp.lang.c++ io_x <a(a)b.c.invalid> wrote: > pheraps i don't understand C or C++; > are you sure that "data.size()" return the max index of x? Standard C++ data containers use the size() member function to tell the amount of elements. > than where "data" came from: is it global? Does it matter? Anyways, since this was clearly a member function implementation, it's safe to assume that 'data' is a member variable (well, I could have used an even better name, such as 'mData', but as I said, it doesn't really matter). > could be this ok for a mXmXm matrix that has the index in the > same range 0..m? > > Value_t* MyClass::findValue(Value_t& value) > {size_t x, y, z, m; > m=sizeof(data)/sizeof(data[0][0][0]); > m=sqrt3(m); // 3 radix > for(x=0; x <m; ++x ) > for(y=0; y<m; ++y) > for(z=0; z<m; ++z) > if(data[x][y][z]==value) > return &data[x][y][z]; > return 0; > } What the function is doing isn't important. The point is how to exit the nested loops most cleanly. > in the code you write, for me nothing is clear > it is not question if "is it easy?", it is question "is it right?" Well, knowing the language is helpful. |