patternphpMinor
Display four random entries from an array
Viewed 0 times
randomfromarrayfourdisplayentries
Problem
This code will take the array shown and display 4 random entries in the format shown (or near enough).
Are there any parts of this that can be optimised? I'm not happy about the
Are there any parts of this that can be optimised? I'm not happy about the
if statement.= 3){
break;
}
$i++;
}
return $random;
}
$arr = array();
$arr[] = array('img' => "http://placehold.it/150x150", 'link' => '#1', 'text1' => "ONE 1", 'text2' => 'ONE 2');
$arr[] = array('img' => "http://placehold.it/200x150", 'link' => '#2', 'text1' => "TWO 1", 'text2' => 'TWO 2');
$arr[] = array('img' => "http://placehold.it/250x150", 'link' => '#3', 'text1' => "THREE 1", 'text2' => 'THREE 2');
$arr[] = array('img' => "http://placehold.it/300x150", 'link' => '#4', 'text1' => "FOUR 1", 'text2' => 'FOUR 2');
$arr[] = array('img' => "http://placehold.it/350x150", 'link' => '#5', 'text1' => "FIVE 1", 'text2' => 'FIVE 2');
$arr[] = array('img' => "http://placehold.it/400x150", 'link' => '#6', 'text1' => "SIX 1", 'text2' => 'SIX 2');
$arr[] = array('img' => "http://placehold.it/450x150", 'link' => '#7', 'text1' => "SEVEN 1", 'text2' => 'SEVEN 2');
foreach(shuffle_assoc($arr) as $key => $value)
{
echo '';
echo ''.$value["link"].'';
echo ''.$value["text1"].'';
echo ''.$value["text2"].'';
echo '';
}
?>Solution
I think the function is not named properly. You are NOT shuffling an associative array. You are shuffling a numerically indexed array (whose values happen to be associative arrays).
As such, I don't see a great need for this function. You could use
Now, assuming you did want to keep such a function, for example to actually get array of values not just keys. I would consider a few things:
This might yield something like:
Note that this function does not preserve keys if passed an associative array. This should probably be the logical behavior in such a case, as "shuffling" an associative array typically makes zero sense. This would however still pick X random values from an associative array.
If one needed to pick X random key/value pairs from an associative array, I would suggest a separate function, so that intent of caller is clear. That function may be implemented in very similar manner to the one above, with single line of code change.
As such, I don't see a great need for this function. You could use
array_rand() to pick random keys from the array.Now, assuming you did want to keep such a function, for example to actually get array of values not just keys. I would consider a few things:
- Consider throwing an exception or logging error if function is passed a non-array value. This can help make sure you are invoking this function properly in your code vs. just returning the passed value unchanged.
- Consider passing "limit" value to the function vs. hard-coding for 4 return elements.
This might yield something like:
function array_rand_values($arr, $limit = 1) {
if(!is_array($arr)) {
throw new InvalidArgumentException('Array expected for first argument.');
}
if(!is_int($limit) || $limit < 1) {
throw new InvalidArgumentException(
'Positive integer value expected for second argument.'
);
}
if(count($arr) <= $limit) {
shuffle($arr);
return $arr;
}
$keys = array_rand($arr, $limit);
$values = array();
for ($i = 0; $i < $limit; $i++) {
$values[] = $arr[$keys[$i]];
}
return $values;
}Note that this function does not preserve keys if passed an associative array. This should probably be the logical behavior in such a case, as "shuffling" an associative array typically makes zero sense. This would however still pick X random values from an associative array.
If one needed to pick X random key/value pairs from an associative array, I would suggest a separate function, so that intent of caller is clear. That function may be implemented in very similar manner to the one above, with single line of code change.
function array_rand_values_assoc($arr, $limit = 1) {
// same code as above until assignment in loop
for ($i = 0; $i < $limit; $i++) {
$values[$keys[$i]] = $arr[$keys[$i]];
}
return $values;
}Code Snippets
function array_rand_values($arr, $limit = 1) {
if(!is_array($arr)) {
throw new InvalidArgumentException('Array expected for first argument.');
}
if(!is_int($limit) || $limit < 1) {
throw new InvalidArgumentException(
'Positive integer value expected for second argument.'
);
}
if(count($arr) <= $limit) {
shuffle($arr);
return $arr;
}
$keys = array_rand($arr, $limit);
$values = array();
for ($i = 0; $i < $limit; $i++) {
$values[] = $arr[$keys[$i]];
}
return $values;
}function array_rand_values_assoc($arr, $limit = 1) {
// same code as above until assignment in loop
for ($i = 0; $i < $limit; $i++) {
$values[$keys[$i]] = $arr[$keys[$i]];
}
return $values;
}Context
StackExchange Code Review Q#153494, answer score: 2
Revisions (0)
No revisions yet.