current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite
1

In five minutes I made a pretty ugly looking function. Can you help before I have to commit the code into history?

Requirements:

I would like a function that takes an array of numbers, and returns an array of ranges. Ideally remove the duplicate line.

Sample Input:

Array(1,2,3,4,5,6,10,11,12,13,20,24)

Output:

Array
(
    [0] => 1-6
    [1] => 10-13
    [2] => 20
    [3] => 24
)

Winning Entries:

Pseudo code  
PHP

My attempt:

$myArray = array(1,2,3,4,5,6,10,11,12,13,20,24);

$rangeArray = array();
$start = $end = current($myArray);
foreach($myArray as $range){
    if($range - $end > 1){
        $rangeArray[] = ($start == $end)?$start:$start."-".$end;
        $start = $range;
    }

    $end = $range;
}
$rangeArray[] = ($start == $end)?$start:$start."-".$end;

print_r($rangeArray);
share|improve this question
    
I don't see a better way to do it more efficiently in an algorithmic meaning. This kind of task must go straight to the point and that is exactly what you have done. However if you project to build a function, you will need to check the parameters, to define default behaviors and to throw exceptions for inappropriate types of data, that's all. –  Casimir et Hippolyte Feb 14 at 10:01

3 Answers 3

up vote 2 down vote

You could add a null value at the end of your main array and remove last line. This way the null will be dropped, but it probably will be harder to understand if someone comes later to see your code.

Maybe better naming for your vars.

I played a bit with the problem, here's what I got. The idea is the same wrote in a different way.

$myArray = array(1,2,3,4,5,6,10,11,12,13,20,24);

//last value is dropped so add something useless to be dropped
array_push($myArray, null);
$rangeArray = array();

array_walk($myArray, function($val) use (&$rangeArray){
    static $oldVal, $rangeStart;

    if (is_null($rangeStart))
        goto init;

    if ($oldVal+1 == $val) {
        $oldVal = $val;
        return;
    }

    if ($oldVal == $rangeStart) {
        array_push($rangeArray, $rangeStart);
        goto init;
    }

    array_push($rangeArray, $rangeStart . '-' . $oldVal);

    init: {
        $rangeStart = $val;
        $oldVal = $val;
    }
});

print_r($rangeArray);
share|improve this answer
    
Thank you for your input. I am not a big fan of the goto statement and therefore usually stay away, with a little bit of refactoring it can be removed. Interesting idea to add a null value to remove the need to duplicate the array_push line. –  William George Feb 11 at 19:36
    
Hi @WilliamGeorge, if you like this (and other answers too, as per your other comments), why not reward them with upvotes? –  janos Feb 15 at 11:45
up vote 2 down vote

more like an alternative to give some impulse than a review of your code

<?php
$myArray = array(1,2,3,4,5,6,10,11,12,13,20,24);
$rangeArray = array();

class ArrayAggregator implements Iterator {

    private $data;
    private $index = 0;

    public function __construct(array $array) {
        $this->data = $array;
    }

    public function current()
    {
        // save the start value
        $startValue = $this->_current();

        // iterate until gap is bigger than 1
        do {
            // get the "real" current value
            $value = $this->_current();
            // move to the next value
            $this->next();
            // until gap
        } while ($this->_current() == $value +1);

        // @todo more grace
        // set index back because foreach will call "next" on after we return the current value
        $this->index -= 1;

        // return start and end value as aggregate output
        return sprintf('%d - %d', $startValue, $value);

    }

    private function _current() {
        return $this->data[$this->index];
    }

    public function next()
    {
        $this->index += 1;
    }

    public function key()
    {
        return $this->index;
    }

    public function valid()
    {
        return isset($this->data[$this->index]);
    }

    public function rewind()
    {
        $this->index = 0;
    }
}

$aa = new ArrayAggregator($myArray);
foreach ($aa  as $a) {
    echo $a."\n";
}
share|improve this answer
    
Wow, thats a fun class. I never would have thought to do that. A little overkill for my specific needs, but I like it :) –  William George Feb 11 at 20:14
up vote 2 down vote

I've changed the output slightly to make the processing easier:

$numbers = array(1,2,3,4,5,6,10,11,12,13,20,24);

$ranges[] = array($numbers[0],$numbers[0]); // initial value

foreach ($numbers as $number) {
  $range    = array_pop($ranges);
  $extend   = ($range[1] == $number-1);
  $ranges[] = array($range[0],$extend ? $number : $range[1]);
  if (!$extend) $ranges[] = array($number,$number);
}

echo '<pre>'.print_r($ranges,TRUE).'</pre>';

It's now easy to use this output for whatever purpose you need it. It looks like this:

Array
(
    [0] => Array
        (
            [0] => 1
            [1] => 1
        )

    [1] => Array
        (
            [0] => 1
            [1] => 6
        )

    [2] => Array
        (
            [0] => 10
            [1] => 13
        )

    [3] => Array
        (
            [0] => 20
            [1] => 20
        )

    [4] => Array
        (
            [0] => 24
            [1] => 24
        )

)

If you really need to, you could get your output by using this code:

foreach ($ranges as $range) {
  $output[] = ($range[0] == $range[1]) ? $range[0] : $range[0].'-'.$range[1];
}

echo '<pre>'.print_r($output,TRUE).'</pre>';

Which will result in:

Array
(
    [0] => 1
    [1] => 1-6
    [2] => 10-13
    [3] => 20
    [4] => 24
)
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.