2
\$\begingroup\$

I want to refactor my code but I can't find and much simplier one. Can you please suggest on how to refactor the code below without using a loop? maybe using only array functions? TIA

<?php
$week_no = array(2,3,4);
$days = array('TU', 'WE', 'TH'); 

foreach ($week_no as $n) {
  foreach ($days as $d) {
   $out[] = $n . $d;
  }
}

var_dump($out); // array('2TU', '2WE', '2TH','3TU', '3WE', '3TH, '4TU', '4WE', '4TH)
?>
\$\endgroup\$
6
  • 4
    \$\begingroup\$ that's the simple and the fastest way to do it !! \$\endgroup\$ – mgraph Sep 6 '12 at 10:30
  • \$\begingroup\$ yes. but is there anymore more elegant than this? :) \$\endgroup\$ – Peter Sep 6 '12 at 10:31
  • \$\begingroup\$ maybe you can change the structure of youre array depends on how you want to use it. \$\endgroup\$ – Jurgo Sep 6 '12 at 10:32
  • \$\begingroup\$ I was working out a version using array_reduce just for shits and giggles, but my hands got tired halfway through... Seriously, this is the simplest way to do it. \$\endgroup\$ – deceze Sep 6 '12 at 10:36
  • \$\begingroup\$ $week_no = array(2,3,4); $days = array('TU', 'WE', 'TH'); function concatenateArrayValues(&$value, $key, $data) { $value = $data[1][floor($key/count($data[1]))] . $data[0][$key % count($data[0])]; } $compositeArray = array_fill(0,count($week_no)*count($days),NULL); array_walk($compositeArray,'concatenateArrayValues',array($days,$week_no)); var_dump($compositeArray); \$\endgroup\$ – Mark Baker Sep 6 '12 at 12:45
4
\$\begingroup\$

What's wrong with a simple:

$out = array('2TU', '2WE', '2TH','3TU', '3WE', '3TH, '4TU', '4WE', '4TH);
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Its repetitive and doesn't allow for extension. Now if you were to add Monday or Friday to that list, you'd have to manually do so for each week. \$\endgroup\$ – mseancole Sep 6 '12 at 17:03
  • \$\begingroup\$ +1 from me, this is the simplest. If there aren't multiple places (at least 3 or more) where these week/days are used then this beats adding other complexity with a function etc. \$\endgroup\$ – Paul Sep 7 '12 at 3:54
  • 2
    \$\begingroup\$ There's no mention in the question of extensions. I'm a pragmatic programmer. The time to add complexity is when the problem becomes more complex, not before ;-) \$\endgroup\$ – RichardAtHome Sep 7 '12 at 8:23
4
\$\begingroup\$

How about this for some madness:

function weekday($a, $b){global $days; return "$a $b".join(" $b", $days);};
$out = explode(' ', trim(array_reduce($week_no, 'weekday')));

(Urm, yes well maybe not...)

Sometimes to attain elegance you have to change the way your code is working, i.e. what are your reasons behind generating such an odd array? From my experience something like this would be more useful:

$out = array_fill(reset($week_no),count($week_no),$days);

Which generates the following:

Array
(
    [2] => Array
        (
            [0] => TU
            [1] => WE
            [2] => TH
        )

    [3] => Array
        (
            [0] => TU
            [1] => WE
            [2] => TH
        )

    [4] => Array
        (
            [0] => TU
            [1] => WE
            [2] => TH
        )

)

The above would be much easier to traverse and would be more extendable.

In sticking with your question however the foreach method is by far the best as stated in the comments... but it was fun trying odd work arounds ;) Am surprised that there is no php function to directly prepend or append array items with another set of array items... probably because it's quite easy and fast to do so with a few foreachs.

\$\endgroup\$
4
  • \$\begingroup\$ I would say using globals is not really an improvement. \$\endgroup\$ – Ikke Sep 6 '12 at 14:24
  • \$\begingroup\$ Yep, totally... that's was the reason for the (erm, yes well maybe not part) ;) Was just an example of the lengths you would have to go to in order to achieve the same... and the reason for me stating a possible implementation change. \$\endgroup\$ – Pebbl Sep 6 '12 at 15:41
  • \$\begingroup\$ +1 For the restructure, not for the globals shudder I was originally debating array_combine(), but this comes out nicer. \$\endgroup\$ – mseancole Sep 6 '12 at 16:13
  • \$\begingroup\$ Thanks, heh, yep Globals are rather evil - It's a shame array_reduce doesn't accept a userdata param like array_walk. However, on the plus side it's the first time I've actually found a use for array_reduce... albeit a rather non-use. The worse part imo though is the conversion to string and then back to array again... (which I was expecting ppl to complain about more) \$\endgroup\$ – Pebbl Sep 6 '12 at 21:22
1
\$\begingroup\$

I found another solution with array_merge and array_map. However it's bigger and probably slower than your foreach-solution.

$out =  call_user_func_array('array_merge',
                array_map(function($a) use ($days){
                        return array_map(function($b) use ($a){
                                return $a . $b;
                        }, $days);
                }, $week_no, $days)
        );
\$\endgroup\$
2
  • \$\begingroup\$ Indentation works just fine if you use the provided formatting functions which are explained in the (not overlookable!) help just above the input field, highlighted by an ugly mustard yellow. \$\endgroup\$ – Konrad Rudolph Sep 7 '12 at 8:12
  • \$\begingroup\$ You're right. I was hung up on backticks as they are meant to highlight code. It's probably just there that indentation doesn't work as expected. Thanks anyway! \$\endgroup\$ – Louis Huppenbauer Sep 7 '12 at 8:15
1
\$\begingroup\$

Not necessarily shorter, but you could create an object that takes the two arrays and does the merge for you, possibly using something like array_walk or array_map.

\$\endgroup\$
1
\$\begingroup\$

Stop. This is as simple as you're going to get it. Any further refactoring will be making your code more complex rather than simpler. Look at the other code suggestions... are they making it simpler or more complex? You may want to wrap this inside a function depending on how your application uses it, but without seeing more code it's impossible to judge.

There are a lot of cases where there is a nice array function to call, but this is not one of those cases. Instead of worrying about how many characters or lines it takes to perform a task, worry about which implementation makes your intent the most clear. That's what matters.

Don't be sad... what you have is perfect.

\$\endgroup\$
0
\$\begingroup\$

The only thing I would do is wrap everything inside a function.

function getWeekArray( $week_no, $days ){
    $result = array();
    foreach ($week_no as $n) {
      foreach ($days as $d) {
       $result[] = $n . $d;
      }
    }
    return $result;
}

$a = array(2,3,4);
$b = array('TU', 'WE', 'TH'); 

var_dump(getWeekArray( $a, $b )); // array('2TU', '2WE', '2TH','3TU', '3WE', '3TH, '4TU', '4WE', '4TH)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy