Function turns into an endless loop

Question

I have a problem with a function I have written in php. As you can see the function uses itself to return an array of the 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 it calls itself to get each of the variables it turns into an endless loop.

What causes this?

Answer 1

You must always think of writing a recursive function in two parts:

1. The base case - at which point do you stop recursing and return a value (i.e. is the list empty)
2. The recursive case - how do you call a function again and how does the input differ from the previous call (i.e. do you send the tail of the list)

Ensuring that these two rules hold should result in a recursive function that terminates given that 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);
 }
}

Answer 2

Can I suggest that perhaps a better 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 to why your function isn't ending, I'm not sure. Normally I'd do what you're trying with two seperate function calls though, like:

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 things easier to read, and more stable besides, just incase PHP interprets something as "array" when it shouldn't.

Answer 3

Its simple really, when you think about it.

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

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

if ($day === 'array')

EDIT

I also took the liberty to fix up 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];
}