From: Dave Harris on 29 Apr 2010 15:45 kst-u(a)mib.org (Keith Thompson) wrote (abridged): > Please look again. I'm fairly sure that all the versions I've seen > in this thread terminate after finding the item. Indeed. But that someone misunderstood what the loop conditions were doing undermines the claim that the return-free code was simpler. for(int xInd = 0; !a && xInd < datasize; ++xInd) I suspect the author uses a single letter variable in order to minimise the visual impact of the extra test, with the result that it got overlooked completely. Had it been: for(int xInd = 0; pResult != nullptr && xInd < datasize; ++xInd) the code would be clearer and the extra complexity more obvious. [Later...] Ah, I see people are also confused about the difference between !a and !*a. Again showing that "a" is not a good variable name here, and also illustrating that when you add spurious mutable state to a routine, you also have to think of a good name for it, and it's possible to get this wrong. -- Dave Harris, Nottingham, UK.
From: Keith Thompson on 29 Apr 2010 15:45 Nick <3-nospam(a)temporary-address.org.uk> writes: > Keith Thompson <kst-u(a)mib.org> writes: >> Jeff Flinn <TriumphSprint2000(a)hotmail.com> writes: >>> Keith Thompson wrote: >>>> Jeff Flinn <TriumphSprint2000(a)hotmail.com> writes: >>>>> Keith Thompson wrote: >>>>>> Seebs <usenet-nospam(a)seebs.net> writes: >>>>>>> On 2010-04-28, bart.c <bartc(a)freeuk.com> wrote: >>>>>>>> int* findvalue(int value) >>>>>>>> {int *a=0; >>>>>>>> >>>>>>>> for(int xInd = 0; !a && xInd < datasize; ++xInd) >>>>>>>> for(int yInd = 0; !a && yInd < dataxindsize; ++yInd) >>>>>>>> for(int zInd = 0; !a && zInd < dataxindyindsize; ++zInd) >>>>>>>> { >>>>>>>> if(data[xInd][yInd][zInd] == value) >>>>>>>> a=&data[xInd][yInd][zInd]; >>>>>>>> } >>>>>>>> >>>>>>>> return a; >>>>>>>> } >>>>>>> This is noticably slower and less clear. I would not consider it >>>>>>> an acceptable rewrite -- but I would consider the other form a good >>>>>>> rewrite of it. >>>>>> I mostly agree, but -- "noticably slower"? How did you notice? >>>>> Certainly is for the case data[0][0][0] == value. ;-) >>>> >>>> How so? >>>> >>>> I'm not arguing that it's not *likely* to be slower, but (a) it's not >>>> likely to be *noticeably* slower unless the entire loop is executed many >>>> many times, and (b) it might not be slower at all, depending on how >>>> clever the compiler is. >>>> >>>> (I'm assuming there's not some semantic difference that I haven't >>>> noticed.) >>> >>> Have you profiled your 'assign address' version versus the previous >>> 'return when found' version? For different sizes? The 'assign address' >>> version is effectively doing a std::for_each, alway visiting every >>> element even if the item found is the 1st one. The 'return when found' >>> version is analogous to std::find_if. For the case data[0][0][0] == >>> value, your algorithm is O(N), while find_if, is O(1). >> >> Please look again. I'm fairly sure that all the versions I've seen in >> this thread terminate after finding the item. > > Not when the item is zero, because that's the "nothing has been found > yet" flag, so the search executes to exhaustion even if the item was > found hours ago. No, the loops test for ``!a''. ``a'' is a pointer, initialized to 0 (NULL). ``data'' is presumably a 3-D array of int. When the item is found, a is set to point to the item; even if the item's value is 0, a is still non-zero (non-NULL). -- 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: Jeff Flinn on 29 Apr 2010 16:50 Dave Harris wrote: > kst-u(a)mib.org (Keith Thompson) wrote (abridged): >> Please look again. I'm fairly sure that all the versions I've seen >> in this thread terminate after finding the item. > > Indeed. But that someone misunderstood what the loop conditions were > doing undermines the claim that the return-free code was simpler. > > for(int xInd = 0; !a && xInd < datasize; ++xInd) > > I suspect the author uses a single letter variable in order to minimise > the visual impact of the extra test, with the result that it got > overlooked completely. Had it been: > > for(int xInd = 0; pResult != nullptr && xInd < datasize; ++xInd) > > the code would be clearer and the extra complexity more obvious. Exactly, I totally missed the !a, and would have - by the name - mistakenly read it as checking the value. jeff
From: Daniel T. on 29 Apr 2010 18:05 brangdon(a)cix.co.uk (Dave Harris) wrote: > kst-u(a)mib.org (Keith Thompson) wrote (abridged): > > > Please look again. I'm fairly sure that all the versions I've seen > > in this thread terminate after finding the item. > > Indeed. But that someone misunderstood what the loop conditions were > doing undermines the claim that the return-free code was simpler. > > for(int xInd = 0; !a && xInd < datasize; ++xInd) > > I suspect the author uses a single letter variable in order to > minimise the visual impact of the extra test, with the result that it > got overlooked completely. Had it been: > > for(int xInd = 0; pResult != nullptr && xInd < datasize; ++xInd) > > the code would be clearer and the extra complexity more obvious. > > [Later...] > > Ah, I see people are also confused about the difference between !a and > !*a. Again showing that "a" is not a good variable name here, and also > illustrating that when you add spurious mutable state to a routine, > you also have to think of a good name for it, and it's possible to get > this wrong. The extra complexity is already obvious in the fact that three for loops are required to iterate through one container in the first place. The conditions in each of the for loops isn't much of a problem at all in comparison.
From: Phil Carmody on 29 Apr 2010 18:22
brangdon(a)cix.co.uk (Dave Harris) writes: > kst-u(a)mib.org (Keith Thompson) wrote (abridged): >> Please look again. I'm fairly sure that all the versions I've seen >> in this thread terminate after finding the item. > > Indeed. But that someone misunderstood what the loop conditions were > doing undermines the claim that the return-free code was simpler. > > for(int xInd = 0; !a && xInd < datasize; ++xInd) > > I suspect the author uses a single letter variable in order to minimise > the visual impact of the extra test, with the result that it got > overlooked completely. Had it been: > > for(int xInd = 0; pResult != nullptr && xInd < datasize; ++xInd) > > the code would be clearer and the extra complexity more obvious. > > [Later...] > > Ah, I see people are also confused about the difference between !a and > !*a. I think that simply proves that those who've made comments about lack of understandability are horrifically ill-equipped to be programmers, and, as such, their point of view is next-to-worthless. > Again showing that "a" is not a good variable name here, and also > illustrating that when you add spurious mutable state to a routine, you > also have to think of a good name for it, and it's possible to get this > wrong. Nope, if you can't tell a from *a, then stop coding now before you put any more bugs in whatever you're doing. Phil -- I find the easiest thing to do is to k/f myself and just troll away -- David Melville on r.a.s.f1 |