Skip to main content
Thanks Corbin, I didn't notice the dot. Fixed now!
Source Link

I agree that your solution is proper and any reviewer would understand that you're experienced.

My suggestion, beyond palacsint's, is that you slim your code down a bit to make it more readable. I cut 11 lines from get, decreasing it's length by 46%, without compromising on readability one bit.

public function get($start=1, $end=100) {
    $data = array();

    for($num = $start; $num <= $end; $num++) {
        foreach($this->fizzBuzz as $period => $text)
            if($num % $period === 0)
                $data[$num] .= $text;
            else
                $data[$num] = $num;
    }

    return $data;
}

Of course this is personal preference, but I hope to inspire thought regardless of whether that would cause an actual change of mind or not.

PS. Sorry to comment on such an old question. I simply wanted to post my thoughts for anyone who might dig this up as I did.

I agree that your solution is proper and any reviewer would understand that you're experienced.

My suggestion, beyond palacsint's, is that you slim your code down a bit to make it more readable. I cut 11 lines from get, decreasing it's length by 46%, without compromising on readability one bit.

public function get($start=1, $end=100) {
    $data = array();

    for($num = $start; $num <= $end; $num++) {
        foreach($this->fizzBuzz as $period => $text)
            if($num % $period === 0)
                $data[$num] .= $text;
            else
                $data[$num] = $num;
    }

    return $data;
}

Of course this is personal preference, but I hope to inspire thought regardless of whether that would cause an actual change of mind or not.

PS. Sorry to comment on such an old question. I simply wanted to post my thoughts for anyone who might dig this up as I did.

I agree that your solution is proper and any reviewer would understand that you're experienced.

My suggestion, beyond palacsint's, is that you slim your code down a bit to make it more readable. I cut 11 lines from get, decreasing it's length by 46%, without compromising on readability one bit.

public function get($start=1, $end=100) {
    $data = array();

    for($num = $start; $num <= $end; $num++) {
        foreach($this->fizzBuzz as $period => $text)
            if($num % $period === 0)
                $data[$num] = $text;
            else
                $data[$num] = $num;
    }

    return $data;
}

Of course this is personal preference, but I hope to inspire thought regardless of whether that would cause an actual change of mind or not.

PS. Sorry to comment on such an old question. I simply wanted to post my thoughts for anyone who might dig this up as I did.

Source Link

I agree that your solution is proper and any reviewer would understand that you're experienced.

My suggestion, beyond palacsint's, is that you slim your code down a bit to make it more readable. I cut 11 lines from get, decreasing it's length by 46%, without compromising on readability one bit.

public function get($start=1, $end=100) {
    $data = array();

    for($num = $start; $num <= $end; $num++) {
        foreach($this->fizzBuzz as $period => $text)
            if($num % $period === 0)
                $data[$num] .= $text;
            else
                $data[$num] = $num;
    }

    return $data;
}

Of course this is personal preference, but I hope to inspire thought regardless of whether that would cause an actual change of mind or not.

PS. Sorry to comment on such an old question. I simply wanted to post my thoughts for anyone who might dig this up as I did.