HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Finding combinations of 3 numbers

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numberscombinationsfinding

Problem

What this code tries to do is find possible combinations of three numbers. For example, let's say I have the following numbers:

1, 2, 3, 4, 5, 6, 7


Some possible combinations of three numbers would be:

1,2,3 
1,2,4 
2,3,4
2,3,5
2,3,6


I do get the desired outcome but my code looks ugly. Are there any remarks or alternate ways to do this?

```
";

foreach ($numbers as $key) {

if (count($number) == 6) {
// $slice = array_slice($number, 0,3);

$threes[] = $key;
$threes[] .= next($number);
$threes[] .= next($number);

$x++;

array_pop($threes);
$threes[] .= next($number);

$x++;

array_pop($threes);
$threes[] .= next($number);

$x++;

array_pop($threes);
$threes[] .= next($number);

$x++;

//--------------------------------------------------------------//

array_pop($threes);
array_pop($threes);
$threes[] .= prev($number);
array_pop($threes);
$threes[] .= prev($number);
array_pop($threes);
$threes[] .= prev($number);
$threes[] .= next($number);

$x++;

array_pop($threes);
$threes[] .= next($number);

$x++;

array_pop($threes);
$threes[] .= next($number);

$x++;

array_pop($threes);
array_pop($threes);
$threes[] = prev($number);
array_pop($threes);
$threes[] = prev($number);
$threes[] = next($number);

$x++;

array_pop($threes);
$threes[] = next($number);

$x++;

array_pop($threes);
array_pop($threes);
$threes[] = prev($number);
$threes[] = next($number);

$x++;

} elseif (count($number) == 5) {

Solution

Your current code

After a long investigation of the problem, I ended up solving more than what you are asking for. I totally misunderstood what your code were actually doing. I found out that your $numbers array is not interesting at all.

The output of your code, the variable $x is the Binomial coefficient for 6 choose 3. (Which is quite funny because in your question you asked about 7 numbers, but in your code you are using 6).

Your code is doing a whole lot of array_pop, next and prev which is just ugly. You need to spend more time thinking about what the algorithm is behind all of this.

Additionally, I don't know what you are intending to do with these lines:

$threes[] .= next($number);


Why use .= ? Last time I checked, .= is for string concatenation. What does that have to do with anything? And why do that when you add a new index to the array? That code does exactly the same as $threes[] = next($number);

You seem to add a whole lot of stuff to the $threes array, but you never end up using it.

Additionally, your $stake, $odds and even $slice seem totally irrelevant here.

Shortening your code

When removing everything related to $threes, and simplifying your multiple $x++ to one $x += ...; your code can be dramatically shortened:

";

    foreach ($numbers as $key) {

        if (count($number) == 6) {
            $x += 10;
        } elseif (count($number) == 5) {
            $x += 6;
        } elseif (count($number) == 4) {
            $x += 3;
        } elseif (count($number) == 3) {
        }
        array_shift($number);
    }

    echo "Total combinations of 3's: ".$x."";

?>


Then, when removing some more fluff and using an integer variable instead of that $numbers array.

= 0; $i--) {
        if ($i == 6) {
            $x += 10;
        } elseif ($i == 5) {
            $x += 6;
        } elseif ($i == 4) {
            $x += 3;
        }
    }

    echo "Total combinations of 3's: ".$x."";

?>


So, your code is not very extensible in it's current form. It can only support 3's and a maximum $number of 6... That's not optimal.

A better approach

This is how I calculate the Binomial coefficient in Java - which is in use in my Minesweeper Flags Extreme project, translated into PHP

function nCr($n, $r) {
    if (($r > $n) || ($r < 0)) {
        return 0;
    }
    if (($r == 0) || ($r == $n)) {
        return 1;
    }
    
    $value = 1;
    
    for ($i = 0; $i < $r; $i++) {
        $value = $value * ($n - $i) / ($r - $i);
    }
    
    return $value;
}

echo nCr(8, 4); // Prints 70
echo nCr(6, 3); // Your code example, prints 20


It is very tempting to write $value *= ..., but avoid doing that as the results will not be the same in languages which separates integer division and floating division (not that PHP is one of those languages, but I would still recommend avoiding it).

Code Snippets

$threes[] .= next($number);
<?php

    $numbers = [2, 8, 16, 30, 44, 48];
    $number  = $numbers;
    $x       = 1;
    echo implode(" | ", $numbers)."<br>";

    foreach ($numbers as $key) {

        if (count($number) == 6) {
            $x += 10;
        } elseif (count($number) == 5) {
            $x += 6;
        } elseif (count($number) == 4) {
            $x += 3;
        } elseif (count($number) == 3) {
        }
        array_shift($number);
    }

    echo "Total combinations of 3's: ".$x."<br>";

?>
<?php

    $number = 4;
    $x       = 1;
    for ($i = $number; $i >= 0; $i--) {
        if ($i == 6) {
            $x += 10;
        } elseif ($i == 5) {
            $x += 6;
        } elseif ($i == 4) {
            $x += 3;
        }
    }

    echo "Total combinations of 3's: ".$x."<br>";

?>
function nCr($n, $r) {
    if (($r > $n) || ($r < 0)) {
        return 0;
    }
    if (($r == 0) || ($r == $n)) {
        return 1;
    }
    
    $value = 1;
    
    for ($i = 0; $i < $r; $i++) {
        $value = $value * ($n - $i) / ($r - $i);
    }
    
    return $value;
}

echo nCr(8, 4); // Prints 70
echo nCr(6, 3); // Your code example, prints 20

Context

StackExchange Code Review Q#62084, answer score: 5

Revisions (0)

No revisions yet.