user11506: In the code below I am using a for loop to iterate over analog pins (potentiometers) and pass their value, after being mapped, to PWM enabled analog output pins (currently controlling LED brightness). While this works as is, I am curious about the following: would it be better to split the analogRead and analogWrite lines into separate for loops, or would that simply be redundant from a code standpoint? My concern is accidentally mixing up the pot values due to difficult-to-perceive timing issues.
The code:
static const uint8_t analog_pins[] = {A0, A1, A2, A3};
static const uint8_t digital_pins[] = {3, 5, 6, 9};
int pot_vals[] = {0, 0, 0, 0};
int val[] = {0, 0, 0, 0};

void setup() {
  Serial.begin(9600);

  // setup inputs, outputs
  for ( int i = 0; i < 4; i++ ) {
    pinMode( analog_pins[i], INPUT );
    pinMode( digital_pins[i], OUTPUT );
  }
}

void loop() {
  for (int i = 0; i < 4; i++) {
    val[i] = analogRead(analog_pins[i]);
    analogWrite(digital_pins[i], map( val[i], 0, 1023, 0, 255 ));
  }
  delay(2);
}


user30005: Obviously you can. Except that in most cases your loop execution is much faster than your own period so that you end up changing the duty cycles multiple times in one period.
Not an advisable aporoach. If you have to do it think about configuring the pwm center aligned.

user7508: Having both the analogRead() and analogWrite() in the same loop is
perfectly fine. That would be be my preferred choice if only for one
reason: less RAM consumption. If you split them in two loops, you have
to store somewhere the readings of the four channels, either before or
after mapping. If you keep them in the same loop, you don't need the
pot_vals[] nor val[] arrays. You would use a local variable instead,
which consumes no RAM:
void loop() {
  for (int i = 0; i < 4; i++) {
    int val = analogRead(analog_pins[i]);
    analogWrite(digital_pins[i], map(val, 0, 1023, 0, 255));
  }
  delay(2);
}


My concern is accidentally mixing up the pot values due to
  difficult-to-perceive timing issues.

“Difficult to perceive”? I would call these issues “imaginary” instead.
You have no risk of “mixing” any values here. Maybe, if you can
elaborate on the kind of issues you imagine you could have, we could
help you clear some misunderstanding about how the microcontroller
executes your code.

Edit: To expand on Andrew's comment, the for loop can be optimized
as
for (int i = 0; i < 4; i++) {
  unsigned int val = analogRead(analog_pins[i]);
  analogWrite(digital_pins[i], val / 4);
}

Dividing the value by 4 is the best way to map the input range to the
output range. It gives a more uniform mapping than the map() call
above, as every output value is mapped to exactly 4 input values. It
could be noted, though, that map(val, 0, 1024, 0, 256) gives the same
uniform mapping.
The division by 4, however, is optimized by the compiler into a bit
shift, which is orders of magnitude faster than map(), and likely to
consume less stack space. There is one caveat to be aware of: this
optimization is only possible if val is unsigned. If it is signed,
then it is compiled into a call to a division routine, which is quite
expensive.
One could be tempted to go even further and get rid of the temporary
val variable, like:
for (int i = 0; i < 4; i++) {
  analogWrite(digital_pins[i], (unsigned int) analogRead(analog_pins[i]) / 4);
}

However, there is zero benefit from doing so: from the point of view of
an optimizing compiler, this form is exactly equivalent to the previous
one. I prefer the former which I deem more readable.

11506: <p>In the code below I am using a <code>for</code> loop to iterate over analog pins (potentiometers) and pass their value, after being mapped, to <code>PWM</code> enabled analog output pins (currently controlling LED brightness). While this works as is, I am curious about the following: would it be better to split the <code>analogRead</code> and <code>analogWrite</code> lines into separate <code>for</code> loops, or would that simply be redundant from a code standpoint? My concern is accidentally mixing up the pot values due to difficult-to-perceive timing issues.</p>

<p>The code:</p>

<pre><code>static const uint8_t analog_pins[] = {A0, A1, A2, A3};
static const uint8_t digital_pins[] = {3, 5, 6, 9};
int pot_vals[] = {0, 0, 0, 0};
int val[] = {0, 0, 0, 0};

void setup() {
  Serial.begin(9600);

  // setup inputs, outputs
  for ( int i = 0; i &lt; 4; i++ ) {
    pinMode( analog_pins[i], INPUT );
    pinMode( digital_pins[i], OUTPUT );
  }
}

void loop() {
  for (int i = 0; i &lt; 4; i++) {
    val[i] = analogRead(analog_pins[i]);
    analogWrite(digital_pins[i], map( val[i], 0, 1023, 0, 255 ));
  }
  delay(2);
}
</code></pre>
 30005: <p>Obviously you can. Except that in most cases your loop execution is much faster than your own period so that you end up changing the duty cycles multiple times in one period.</p>

<p>Not an advisable aporoach. If you have to do it think about configuring the pwm center aligned.</p>
 7508: <p>Having both the <code>analogRead()</code> and <code>analogWrite()</code> in the same loop is
perfectly fine. That would be be my preferred choice if only for one
reason: less RAM consumption. If you split them in two loops, you have
to store somewhere the readings of the four channels, either before or
after mapping. If you keep them in the same loop, you don't need the
<code>pot_vals[]</code> nor <code>val[]</code> arrays. You would use a local variable instead,
which consumes no RAM:</p>



<pre class="lang-cpp prettyprint-override"><code>void loop() {
  for (int i = 0; i &lt; 4; i++) {
    int val = analogRead(analog_pins[i]);
    analogWrite(digital_pins[i], map(val, 0, 1023, 0, 255));
  }
  delay(2);
}
</code></pre>

<blockquote>
  <p>My concern is accidentally mixing up the pot values due to
  difficult-to-perceive timing issues.</p>
</blockquote>

<p>“Difficult to perceive”? I would call these issues “imaginary” instead.
You have no risk of “mixing” any values here. Maybe, if you can
elaborate on the kind of issues you imagine you could have, we could
help you clear some misunderstanding about how the microcontroller
executes your code.</p>

<hr>

<p><strong>Edit</strong>: To expand on Andrew's comment, the <code>for</code> loop can be optimized
as</p>

<pre class="lang-cpp prettyprint-override"><code>for (int i = 0; i &lt; 4; i++) {
  unsigned int val = analogRead(analog_pins[i]);
  analogWrite(digital_pins[i], val / 4);
}
</code></pre>

<p>Dividing the value by 4 is the best way to map the input range to the
output range. It gives a more uniform mapping than the <code>map()</code> call
above, as every output value is mapped to exactly 4 input values. It
could be noted, though, that <code>map(val, 0, 1024, 0, 256)</code> gives the same
uniform mapping.</p>

<p>The division by 4, however, is optimized by the compiler into a bit
shift, which is orders of magnitude faster than <code>map()</code>, and likely to
consume less stack space. There is one caveat to be aware of: this
optimization is only possible if <code>val</code> is unsigned. If it is signed,
then it is compiled into a call to a division routine, which is quite
expensive.</p>

<p>One could be tempted to go even further and get rid of the temporary
<code>val</code> variable, like:</p>

<pre class="lang-cpp prettyprint-override"><code>for (int i = 0; i &lt; 4; i++) {
  analogWrite(digital_pins[i], (unsigned int) analogRead(analog_pins[i]) / 4);
}
</code></pre>

<p>However, there is zero benefit from doing so: from the point of view of
an optimizing compiler, this form is exactly equivalent to the previous
one. I prefer the former which I deem more readable.</p>
