A way to neaten this PHP code

Question

I have the following PHP code:

<?php

//code above    
$pic_1 = $afil['image_1'];  
$pic_2 = $afil['image_2'];
$pic_3 = $afil['image_3'];
$pic_4 = $afil['image_4'];
$pic_5 = $afil['image_5'];
$pic_6 = $afil['image_6'];
$pic_7= $afil['image_7'];
$pic_8 = $afil['image_8'];
$pic_9 = $afil['image_9'];
$pic_10 = $afil['image_10'];

if ($pic_1 = "")
{
$pic_1 = //defaultpic - to be defined, same as below
}

if ($pic_2 = "")
{
$pic_2 = //defaultpic
}
?>

Rather than repeat these "if" statements for each picture (up until $pic 10) i just wondered if somebody could point out a more elegant and efficient way of doing it. I am quite new to php and this is a new situation i have encountered. Thanks a lot in advance.

You should probably review the manual pages on [control structures](http://php.net/language.control-structures). There, you will find information on how to construct loops using `while`, `for` and most interestingly here, `foreach`. — Charles, Dec 03 '12 at 20:26
Use arrays more effectively. Change your array keys to `$afil['image'][1]` and loop over the `$afil['image']` array. — Frank Farmer, Dec 03 '12 at 20:27
This is a good question; maybe the code you're starting with isn't the best, but that's how you learn. I don't understand why someone would downvote your question. But then, everything here is being downvoted rampantly, so... maybe someone hates PHP. — JYelton, Dec 03 '12 at 20:32

Teena Thomas · Accepted Answer · 2012-12-03T21:09:12.447

4

Use arrays and loop through them with just 1 if statement, like this,

 foreach($afil as $k => $v) {
  if(empty($v))
    $afil[$k] = ...//default pic
 }

Or, if you are keen to have an additional array $pics (for future use maybe),

foreach($afil as $k => $v) {
  $pics[$k] = $v;
  if(empty($v))
    $pics[$k] = ...//default pic
 }

Also, = is an assignment operator. For comparison (or condition check), you need to use == or === (type safe).

Edit:

 $afil = mysql_query(...);
 while($row = mysql_fetch_array($afil)) {
  //In here, say you create an array $result_rows with all the rows
  $result_rows[] = $row;
 }

Then, use $result_rows in the foreach.

edited Dec 03 '12 at 21:09

answered Dec 03 '12 at 20:28

Teena Thomas

5,139
1
13
17

I'm about 60% sure that if you want to change `$v` within the loop and have it reflected in the original array, you'll want to make it a reference. Like `foreach ($afil as $k => &$v) {` – cHao Dec 03 '12 at 20:38
i get an invalid argument? for the "foreach" function. Any idea why? – Ross Dec 03 '12 at 20:45
sorry same applies to you. I'll see if your edit works. Cheers – Ross Dec 03 '12 at 20:46
still says my argument is invalid? – Ross Dec 03 '12 at 20:50
do a `print_r($afil);` right before the loop and see if `$afil` is indeed an array, and all its contents are as expected. – Teena Thomas Dec 03 '12 at 20:51
i get absolutely nothing on that. I probably should point out that $afil = mysql_query.....(in which i am selecting a row from mysql). After which i was then trying to define the local variable as shown above and that was when i wanted a neat way of looping through them. Still $afil is an array isn't it? So don't know why it doesn't work – Ross Dec 03 '12 at 21:02
There you go :), `$afil` is a resource, and NOT an array, if its assigned as `$afil = mysql_query(...)`. I will edit my answer on how to loop through in this case. – Teena Thomas Dec 03 '12 at 21:06

score 0 · Answer 2 · answered Dec 03 '12 at 20:29

0

Arrays is your golden solution:

$pic = array('image_1', 'image_2', 'image_3'....);

for ($i = 0; $i < count($pic); $i++){
if ($pic[$i] == ""){
$pic[$i] = //defaultpic - 
}

}

answered Dec 03 '12 at 20:29

SaidbakR

13,303
20
101
195

Hi,thanks for the suggestion. Firstly i changed the 'image_1" and so to "$pic_1" because "image_1" was coming from a mysql_query. Anyway according to your code the following should be correct shouldn't - $pic[0] = $pic_1 = "uploads/default.png". I then have in the right html tag – Ross Dec 03 '12 at 21:56

A way to neaten this PHP code

2 Answers2