The function turns into an endless loop

I have a problem with a function I wrote in php. As you can see, the function uses itself to return an array of values.

    public function getRepeat($day = "array")
{
    if ($day == 'array')
    {//Return an array with the repeated days as values
        foreach (array(1,2,3,4,5,6,0) as $value) 
        {
            if ($this->getRepeat($value))
            {
                $returnArray[] = $value;
            }
        }
        return $returnArray;
    }
    else if (in_array($day, array(1,2,3,4,5,6,0) ))
    {
        if ($day == 1)
            return $this->repeat1;
        if ($day == 2)
            return $this->repeat2;
        if ($day == 3)
            return $this->repeat3;
        if ($day == 4)
            return $this->repeat4;
        if ($day == 5)
            return $this->repeat5;
        if ($day == 6)
            return $this->repeat6;
        if ($day == 0)
            return $this->repeat0;
    }
}

      

As soon as he calls himself to get each of the variables, he turns into an endless loop.

What is causing this?

+2


source to share


3 answers


You should always think about writing a recursive function in two parts:

  • Base register - at this point you stop recursion and return a value (i.e. an empty list)
  • Recursive case - how do you call the function again and how the input differs from the previous call (i.e. you send the tail of the list)

Enforcing these two rules should result in a recursive function that exits assuming the input is valid.



Here's a recursive solution - however it's in Java :)

    public static void main(String[] args) {

    List<Integer> testVals = new ArrayList<Integer>();
    testVals.add(0);
    testVals.add(1);
    testVals.add(2);
    testVals.add(3);
    testVals.add(4);
    testVals.add(5);

    List<Integer> toMatch = new ArrayList<Integer>(testVals);

    List<Integer> matches = new ArrayList<Integer>();

    repeatRec(testVals, matches, toMatch);

    System.out.println("Matches " + matches);
}

public static void repeatRec(List<Integer> toTest, List<Integer> matches, List<Integer> toMatch) {


    if (toTest.isEmpty()) {
        //we are done
        return;
    } else {

        Integer head = toTest.get(0);

        if (toMatch.contains(head)) {
            matches.add(head);

        }

        //could have else here if we're only interested in the first match
        repeatRec(toTest.subList(1, toTest.size()), matches, toMatch);
    }
}

      

+5


source


Simple when you think about it.

0 == 'any text which does not start with a number'

      

Your last digit 0 will cause an infinite loop. So you need to change it to

if ($day === 'array')

      



EDIT

I also took the liberty of correcting your code:

/**
 * @obsolete
 */
public function getRepeat($day = "array")
{
    if ($day === 'array') {
     return $this->getAllRepeat();
}
    return $this->getRepeatByDay($day);

}

public function __construct()
{
    $this->repeat = array_fill(0, 7, '');
}

public function getAllRepeat()
{
    return $this->repeat;
}

public function __get($value) {
    switch ($value) {
        case 'repeat0':
        case 'repeat1':
        case 'repeat2':
        case 'repeat3':
        case 'repeat4':
        case 'repeat5':
        case 'repeat6':
            return $this->getRepeatByDay(intval(substr($value, -1, 1)));
    }
}

public function getRepeatByDay($day)
{
    if (!isset($this->repeat[$day])) {
        return null;
    }
    return $this->repeat[$day];
}

      

+3


source


May I suggest that perhaps the best solution is:

public function getRepeat($day = "array")
{
    foreach (array(1,2,3,4,5,6,0) as $value) 
    {
        $tmp = "repeat".$value;
        if ($this->$tmp)
        {
            $returnArray[] = $value;
        }
    }
    return $returnArray;
}

      

As for why your function doesn't end, I'm not sure. Usually I would do what you are trying with two separate function calls, for example:

public function getRepeat()
{
                foreach (array(1,2,3,4,5,6,0) as $value) 
                {
                        if ($this->getRepeat_r($value))
                        {
                                $returnArray[] = $value;
                        }
                }
                return $returnArray;
}
private function getRepeat_r($day)
{
        if (in_array($day, array(1,2,3,4,5,6,0) ))
        {
                if ($day == 1)
                        return $this->repeat1;
                if ($day == 2)
                        return $this->repeat2;
                if ($day == 3)
                        return $this->repeat3;
                if ($day == 4)
                        return $this->repeat4;
                if ($day == 5)
                        return $this->repeat5;
                if ($day == 6)
                        return $this->repeat6;
                if ($day == 0)
                        return $this->repeat0;
        }
}

      

This makes it easier to read and also more stable, just interprets PHP as something like "array"

when it shouldn't.

0


source







All Articles