Seek help with this simple PHP array code

Hi,

May I know if the below code is wrote efficiently or correctly? Do I need to close anything? Will it cause excessive php processes? Thanks a lot.


<?php
$divalignrand= array("left", "right");
$divalign1=$divalignrand[rand(0,1)];
$divalign2=$divalignrand[rand(0,1)];
$divalign3=$divalignrand[rand(0,1)];

$bordercolorrand= array("#000", "#FFF", "#F00", "#FF0", "#0F0", "#00F", "#0FF");
$bordercolor1=$bordercolorrand[rand(0,6)];
$bordercolor2=$bordercolorrand[rand(0,6)];
$bordercolor3=$bordercolorrand[rand(0,6)];

$borderstylerand= array("solid", "dashed", "dotted", "double", "groove", "ridge", "inset", "outset");
$borderstyle1=$borderstylerand[rand(0,7)];
$borderstyle2=$borderstylerand[rand(0,7)];
$borderstyle3=$borderstylerand[rand(0,7)];

$borderwidthrand= array("thin", "medium", "thick");
$borderwidth1=$borderwidthrand[rand(0,2)];
$borderwidth2=$borderwidthrand[rand(0,2)];
$borderwidth3=$borderwidthrand[rand(0,2)];

$brrand= array("<BR>", "");
$br1=$brrand[rand(0,1)];
$br2=$brrand[rand(0,1)];
$br3=$brrand[rand(0,1)];
?>

The only two things that stick out to me is that the code doesn’t automatically adjust itself to the size of the options array. Using array_rand works for this, and the repetition. I would either do it the way logic_earth put it (though maybe with an additional parameter, numboxes, to adjust the number of boxes returned…

or alternately just make the function return 1 box, and call it however many number of times necessary (quasi-object style)

Nothing sticks out to me. I don’t see anything that would “open” anything such as a file or database connection. Nothing in there to spawn extra processes either. I’m sure if I toiled over it for awhile I could save a fraction of a millisecond somewhere, but I don’t think it would be worth the effort.

I honsestly would move it into a function and put the result into an array…For example:


function randboxes ( $align, $color, $border, $width, $break )
{
  $ret = array();
  
  for ( $i = 0; $i < 3; $i++ ) {
    $ret[] = array(
      'align'  => $align[ array_rand( $align ) ],
      'color'  => $color[ array_rand( $color ) ],
      'border' => $border[ array_rand( $border ) ],
      'width'  => $width[ array_rand( $width ) ],
      'break'  => $break[ array_rand( $break ) ]
    );
  }
  
  return $ret;
}
  
$align  = array( 'left', 'right' );
$color  = array( '#000', '#FFF', '#F00', '#FF0', '#0F0', '#00F', '#0FF' );
$border = array( 'solid', 'dashed', 'dotted', 'double', 'groove', 'ridge', 'inset', 'outset' );
$width  = array( 'thin', 'medium', 'thick' );
$break  = array( '<br>', '' );
  
$boxes = randboxes( $align, $color, $border, $width, $break );
// Result:
//   $boxes = array(
//     0 => [BOX ONE]
//     1 => [BOX TWO]
//     2 => [BOX THREE]
//   )


Efficiently? well, did you test it out? Premature optimisation is in itself a problem. Don’t try to optimise it unless its needed.

As for ‘correctly’. This is largely subjective. You may want to specify the problem you are trying to address first, as there might be a more idiomatic approach in PHP. For example, you could look at the array_rand() function in PHP which would get rid of some of the clutter you have going on there.

As for “Do I need to close anything” … well did you try and run it? if you have some basic error reporting setup this will pick it up for you.