www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Foreach loop behaviour and manipulation

reply "Binarydepth" <binarydepth gmail.com> writes:
Hi guys I'm having some problems. Calculations are not working as 
expected and I get segmentation fault. I used the 2.059 version 
and it runs after compilation on compileonline.com

But I imagine is either a rookie mistake or a bug, what I'm 
curious about is the foreach loop.

I'm wondering in the case of manipulating the variable from the 
foreach loop, Do I have to reset that variable so the loop can 
work as intended ?(chronologically).

If you don't get what I'm saying check for the comment on the 
code below.

Code :
------------------------------------------------------------------------
import std.stdio : write, readf;
void main()
{
int a, r, f, temp;
int[102] arr;
write("Digite su año de nacimiento : ");
readf(" %d", &a);
write("\n");
foreach(t; 1..51)
     {
	temp=t;
	t*=20;
	t+=420;
	t*=5;
	t+=3;
     arr[temp-1]=t-a;
	t=temp;
	t*=5;
	t+=50;
	t*=20;
	t+=1013;
     arr[temp]=t-a;
     t=temp;//Do I need this for the foreach loop to work as 
intended ?
	}
write("BD\tAnonimo\n");
foreach(count; 0..102)
     {
         write(arr[count]);
         if(count%2==0)
         write("\n");
         else
         write(" : ");
     }
}
-------------------------------------------------------------------------------
Nov 28 2013
next sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Fri, Nov 29, 2013 at 12:36:18AM +0100, Binarydepth wrote:

 Hi guys I'm having some problems. Calculations are not working as
 expected and I get segmentation fault. I used the 2.059 version and
 it runs after compilation on compileonline.com


[...]

 foreach(t; 1..51)
     {
 	temp=t;
 	t*=20;


Modifying the loop variable of a foreach is, in general, a risky move.
If you need to make loop indices jump around, you should use a plain for
loop instead:

	for (t=1; t < 51; t++)
	{
		// modify t at will, just make sure your loop
		// condition(s) / loop increments still work correctly.
	}

or, if the loop indices are truly wildly jumping around, use a while
loop:

	t = 1;
	while (t < 51 /* or whatever condition you may have */)
	{
		... // Do stuff
		t = ... // compute next index to jump to
	}


T

-- 
The computer is only a tool. Unfortunately, so is the user. -- Armaphine, K5
Nov 28 2013
next sibling parent "Binarydepth" <binarydepth gmail.com> writes:
On Thursday, 28 November 2013 at 23:45:26 UTC, H. S. Teoh wrote:

 On Fri, Nov 29, 2013 at 12:36:18AM +0100, Binarydepth wrote:

 Hi guys I'm having some problems. Calculations are not working 
 as
 expected and I get segmentation fault. I used the 2.059 
 version and
 it runs after compilation on compileonline.com


 [...]

 foreach(t; 1..51)
     {
 	temp=t;
 	t*=20;


 Modifying the loop variable of a foreach is, in general, a 
 risky move.
 If you need to make loop indices jump around, you should use a 
 plain for
 loop instead:

 	for (t=1; t < 51; t++)
 	{
 		// modify t at will, just make sure your loop
 		// condition(s) / loop increments still work correctly.
 	}

 or, if the loop indices are truly wildly jumping around, use a 
 while
 loop:

 	t = 1;
 	while (t < 51 /* or whatever condition you may have */)
 	{
 		... // Do stuff
 		t = ... // compute next index to jump to
 	}


 T


Thank you for you response! That's exactly what I was thinking.  
It can be really chaotic to make that mistake in a for loop. But 
it can be fixed with a temporary variable and reset the value of 
the counter at the end of the execution of the loop just before 
it gets incremented.

Here is the C version of this program. I made a mistake the first 
time I made this version. I left the function as an INT function 
and didn't return any value.

On the D version the compiler warned me about that which makes a 
lot of sense. I liked that! :D .

S I went and fixed both codes but the D compiler is not doing a 
good compilation and i get segmentation fault when I run the D 
version.

Finally here's the C code that gives the correct output :
----------------------------------------------------------------------------
#include <stdio.h>
void funcion(int a, int t)
{
int temp, bd, an;
temp=t;
	temp*=20;
	temp+=402;
	temp*=5;
	temp+=3;
bd=temp-a;
temp=t;
	temp*=5;
	temp+=50;
	temp*=20;
	temp+=1013;
an=temp-a;
printf("%d:%d\n", bd, an);
}
void main()
{
int r, f, count, b;
printf("Input your birth year : ");
scanf("%d", &b);

scanf("%d %d", &r, &f);
for(count=r; count<=f; count++)
	{
		funcion(b, count);
	}
}
--------------------------------------------------------------
Nov 28 2013
prev sibling next sibling parent reply "bearophile" <bearophileHUGS lycos.com> writes:
H. S. Teoh:


 Modifying the loop variable of a foreach is, in general, a 
 risky move.


Right. In my opinion D programmers should learn that this is the 
preferred idiom of using foreach ranged loops:

foreach (immutable i; 0 .. n) {}

If a D programmers really needs to modify the index inside the 
loop body, then it's better to use a for() loop, that makes it 
much more explicit and clear what's happening.

Bye,
bearophile
Nov 28 2013
parent "Maxim Fomin" <maxim maxim-fomin.ru> writes:
On Friday, 29 November 2013 at 00:41:16 UTC, bearophile wrote:

 H. S. Teoh:


 Modifying the loop variable of a foreach is, in general, a 
 risky move.


 Right. In my opinion D programmers should learn that this is 
 the preferred idiom of using foreach ranged loops:

 foreach (immutable i; 0 .. n) {}

 Bye,
 bearophile


Unfortunately if you iterate over delegates, this may break 
immutability.

See 
http://forum.dlang.org/thread/felqszcrbvtrepjtfpul forum.dlang.org?page=1
Dec 02 2013
prev sibling parent reply "Binarydepth" <binarydepth gmail.com> writes:
On Thursday, 28 November 2013 at 23:45:26 UTC, H. S. Teoh wrote:

 On Fri, Nov 29, 2013 at 12:36:18AM +0100, Binarydepth wrote:

 Hi guys I'm having some problems. Calculations are not working 
 as
 expected and I get segmentation fault. I used the 2.059 
 version and
 it runs after compilation on compileonline.com


 [...]

 foreach(t; 1..51)
     {
 	temp=t;
 	t*=20;


 Modifying the loop variable of a foreach is, in general, a 
 risky move.
 If you need to make loop indices jump around, you should use a 
 plain for
 loop instead:

 	for (t=1; t < 51; t++)
 	{
 		// modify t at will, just make sure your loop
 		// condition(s) / loop increments still work correctly.
 	}

 or, if the loop indices are truly wildly jumping around, use a 
 while
 loop:

 	t = 1;
 	while (t < 51 /* or whatever condition you may have */)
 	{
 		... // Do stuff
 		t = ... // compute next index to jump to
 	}


 T


This excersice is an example :

"Escriba un programa que determine los números (de cantidad de 
cifras par) divisores de 11
aplicando el siguiente concepto: cuando la suma de los dígitos 
alternos del número son iguales,
ese número es exactamente divisible por once. Por ejemplo 5841: 5 
+ 4 = 8 + 1, por lo tanto
el número 5841 es divisible por once."

The important part is "los números (de cantidad de cifras par)" 
The number of pair digits 10 to 99,  1000 to 9999, 100000 to 
999999, etc...

I immediately thought of this thread. Surely you can use a series 
of if inside the loop that will make it jump the undesired 
numbers.

or ... for(count=1;count>0 && count<100 || count>999 && 
count<10000 || etc...; count++ )
Jan 07 2014
parent reply "Binarydepth" <binarydepth gmail.com> writes:
 or ... for(count=1;count>0 && count<100 || count>999 && 
 count<10000 || etc...; count++ )


mmmm That won't work. It's better separate foreach loops.
Jan 07 2014
parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Tue, Jan 07, 2014 at 09:38:34PM +0000, Binarydepth wrote:

 

or ... for(count=1;count>0 && count<100 || count>999 &&
count<10000 || etc...; count++ )


 
 mmmm That won't work. It's better separate foreach loops.


Y'know, this code snippet really reminds me of why Jackson Structured
Programming helped me so much. While there are many ways of writing
loops, most ways are wrong (some blatantly so, while others only subtly
so). Writing a correct loop requires that the structure of the code
matches the structure of the problem that it's trying to process.

If you're trying to loop over two distinct ranges, 0 to 100 and 999 to
10000, then conceptually they are two different operations, and
therefore should be mapped to two different loops. Trying to combine
them into one usually creates a mess, and even when you get it right,
the resulting code is fragile, error-prone, and hard to maintain.

I often see code like this:

	bool first_time = false;
	for (i=0; i < n; i++) {
		if (first_time) {
			do_something();
			first_time = true;
		}

		if (i+1 == n) { // last time
			do_something_else();
		}

		do_other_things();
	}

This kind of code shows a poor mapping of code structure to problem
structure, which means it's prone to boundary condition bugs, overlap
bugs, and other sorts of problems, not to mention long-range
interdependencies that makes the code basically impossible to reuse, and
hard to maintain.

Consider what the loop is trying to do, by unrolling it. It looks like
this:

	do_something();
	do_other_things();
	do_other_things();
	do_other_things();
	...
	do_something_else();
	do_other_things();

If you "refactor" this, you see that it follows the structure:

	A --> B (repeat k times) --> C --> B

where A = do_something(), B = do_other_things(), and C =
do_something_else().

So you see that the repeating part in the structure of the problem is
really only with the middle part; A and the final C --> B should be put
*outside* the loop body, like this:

	do_something();
	for (i=1; i+1 < n; i++) {	// N.B.: loop bounds adjusted
		do_other_things();
	}
	do_something_else();
	do_other_things();

Many programmers, esp. those with C/C++ background, cringe when they see
the second call to do_other_things() after the loop body, and they try
various ways of putting it inside the loop body -- especially when
do_other_things() is a big code block. But actually, this way of writing
it is the correct way, because it reflects the structure of the problem
more accurately. Putting everything inside one big loop creates a
structure conflict with the problem, which has a different structure;
this invites people to introduce boolean flags and other such hacks to
make things work correctly. But actually, that's just stitching over the
symptoms of the deeper problem, which is that the structure of the code
fails to correspond with the structure of the problem.

By making the structure of the code match the structure of the problem,
we eliminate the proliferation of boolean flags and convoluted loop
conditions (which are very error-prone and basically impossible to
maintain), and the code begins to speak for itself, because just by
looking at the structure of the code, you know what's the structure of
the problem it's working on. Once the structure is properly sorted out,
then we can worry about other things, like do_other_things() being a big
copy-n-pasted code block -- which then basically suggests the obvious
solution: factor it into a function.

A classic example of poor mapping of code structure to problem structure
is join(): inserting (n-1) delimiters into a list of n items. I almost
always see code like this:

	foreach (i, e; range) {
		result.put(e);
		if (i+1 < range.length)
			result.put(',');
	}

But let's step back for a moment and look at what's the *real* structure
of the problem. Suppose range = [1,2,3,4,5]. Then the desired output is:

	1 , 2 , 3 , 4 , 5

Or, more abstractly, if we use 'e' in a generic sense to refer to any
item:

	e , e , e , e , e , e

which can be "factored" into the form:

	(e ,){(n-1) times} e

Due to the nature of the range API, however, this does not lend itself
to a straightforward implementation (there is no primitive for looping
over (n-1) items in a range). So we consider an alternative
factorization:

	e (, e){(n-1) times}

This, then, suggests that the first element of the range should be
treated specially, which leads to the following code:

	if (range.empty) return; // boundary case

	result.put(range.front);
	range.popFront();

	while (!range.empty) {
		result.put(',');
		result.put(range.front);
		range.popFront();
	}

This eliminates the if-statement with a tricky condition: should it be
(i+1 < range.length) or (i <= range.length) or (i < range.length-1)?
While this particular example is trivial to sort out, you'd be surprised
how often I run into code where the condition is wrongly-written,
resulting in off-by-1 errors. By getting rid of the condition
altogether, we eliminate this source of bugs (not to mention the code is
now free from the arbitrary requirement that the range must have a
.length primitive).  Plus, the code now more accurately reflects the
structure of the output.

Some people will object that the above code looks "more verbose",
"ugly", etc., but the fact of the matter is that it reflects the
structure of the problem more accurately than any of the cuter hacks
that tries to "reduce code bloat" by arbitrarily squashing
non-corresponding structures into a single loop, thus resulting in
poorly-motivated if-statements, needlessly complex loop conditions, and
other ad hoc hacks to patch over the structure conflict. The result is
often fragile and hard to maintain. Not to mention, having loops with
trivial conditions gives the optimizer a far easier time at generating
superior code than a loop with a condition too complex for the optimizer
to understand.

Now, there *are* cases where complex loop conditions are necessary, but
I'd say that 90% of loops don't need this and can be (and should be)
reduced to loops with trivial conditions. So whenever your loop
conditions start acquiring lots of &&'s and ||'s, that's a sign
indicating that your code structure no longer corresponds with the
problem structure, and it's time to rewrite your loops.

</soapbox> :-)


T

-- 
Those who don't understand Unix are condemned to reinvent it, poorly.
Jan 07 2014
prev sibling next sibling parent "Binarydepth" <binarydepth gmail.com> writes:
I fixed a formatting problem on the code just now. And also a 
note is that the numbers are supposed to display a number 
depending on the initial value of the variable that the foreach 
loop is increasing and the users age.

So if you were born in 1977 you input your birth year to the 
program and you'll get :

136:136
236:236
336:336
436:436
536:536
...
1036:1036
1136:1136
1236:1236
...
2036:2036
2136:2136
....
5036:5036
end

------------------------------------------------------------------------
import std.stdio : write, readf;
void main()
{
int a, r, f, temp;
int[102] arr;
write("Digite su año de nacimiento : ");
readf(" %d", &a);
write("\n");
foreach(t; 1..51)
     {
	temp=t;
	t*=20;
	t+=402;
	t*=5;
	t+=3;
     arr[temp-1]=t-a;
	t=temp;
	t*=5;
	t+=50;
	t*=20;
	t+=1013;
     arr[temp]=t-a;
     t=temp;//Do I need this for the foreach loop to work as 
intended ?
	}
write("BD\tAnonimo\n");
foreach(count; 0..102)
     {
         write(arr[count]);
         if(count%2!=0 && count!=0)
         write("\n");
         else
         write(" : ");
     }
}
-------------------------------------------------------------------------------
Nov 28 2013
prev sibling parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 29/11/13 00:36, Binarydepth wrote:

 I'm wondering in the case of manipulating the variable from the foreach loop,
Do
 I have to reset that variable so the loop can work as intended
?(chronologically).


I think that you are approaching this problem in the wrong way.  Instead of 
using a temporary variable to preserve the loop value, why not keep the loop 
value constant (immutable, in fact) and use a temporary variable to calculate 
the values that you wish to write to the array?

Like so:

////////////////////////////////////////////////////////////////////////
import std.stdio : write, readf;
void main()
{
     int a, r, f;
     int[102] arr;
     write("Digite su año de nacimiento : ");
     readf(" %d", &a);
     write("\n");
     foreach (immutable t; 1 .. 51)
     {
         int temp = (((t * 20) + 420) * 5) + 3;
         arr[t - 1] = temp - a;
         temp = (((t * 5) + 50) * 20) + 1013;
         arr[t] = temp - a;
     }
     write("BD\tAnonimo\n");
     foreach(count; 0..102)
     {
         write(arr[count]);
         if(count%2==0)
             write("\n");
         else
             write(" : ");
     }
}
////////////////////////////////////////////////////////////////////////

This gives same results as your existing code but (to me at least) is much 
simpler and easier to follow.

2nd thing -- do I assume right that you are getting incorrect output and that 
the later output values shouldn't be all zero?  I think this is because you are 
incorrectly choosing array indexes to write to, but we should perhaps talk
about 
what your program is _supposed_ to do before addressing that.

Am I also right to assume that you're used to languages where the array index 
starts from 1 rather than from 0?

Best wishes,

     -- Joe
Nov 29 2013
next sibling parent "Binarydepth" <binarydepth gmail.com> writes:
On Friday, 29 November 2013 at 09:15:28 UTC, Joseph Rushton 
Wakeling wrote:

 On 29/11/13 00:36, Binarydepth wrote:

 I'm wondering in the case of manipulating the variable from 
 the foreach loop, Do
 I have to reset that variable so the loop can work as intended 
 ?(chronologically).


 I think that you are approaching this problem in the wrong way.
  Instead of using a temporary variable to preserve the loop 
 value, why not keep the loop value constant (immutable, in 
 fact) and use a temporary variable to calculate the values that 
 you wish to write to the array?

 Like so:

 ////////////////////////////////////////////////////////////////////////
 import std.stdio : write, readf;
 void main()
 {
     int a, r, f;
     int[102] arr;
     write("Digite su año de nacimiento : ");
     readf(" %d", &a);
     write("\n");
     foreach (immutable t; 1 .. 51)
     {
         int temp = (((t * 20) + 420) * 5) + 3;
         arr[t - 1] = temp - a;
         temp = (((t * 5) + 50) * 20) + 1013;
         arr[t] = temp - a;
     }
     write("BD\tAnonimo\n");
     foreach(count; 0..102)
     {
         write(arr[count]);
         if(count%2==0)
             write("\n");
         else
             write(" : ");
     }
 }
 ////////////////////////////////////////////////////////////////////////

 This gives same results as your existing code but (to me at 
 least) is much simpler and easier to follow.

 2nd thing -- do I assume right that you are getting incorrect 
 output and that the later output values shouldn't be all zero?  
 I think this is because you are incorrectly choosing array 
 indexes to write to, but we should perhaps talk about what your 
 program is _supposed_ to do before addressing that.

 Am I also right to assume that you're used to languages where 
 the array index starts from 1 rather than from 0?

 Best wishes,

     -- Joe


Completely agree with you. That's actually what I ended up doing. 
:)
Dec 02 2013
prev sibling parent "Binarydepth" <binarydepth gmail.com> writes:
     foreach (immutable t; 1 .. 51)
     {
         int temp = (((t * 20) + 420) * 5) + 3;
         arr[t - 1] = temp - a;
         temp = (((t * 5) + 50) * 20) + 1013;
         arr[t] = temp - a;
     }


Good work with the parenthesis. :)
Dec 02 2013