0

Problem

I am aware that somewhere in my function, I am not returning something I should.

I am returning the recursive call, but it appears that I am not returning "all the way out"

Context

I am doing a depth first search of every single combination in a list. Once I reach a combination that reaches a condition, I want to return.

I am maintaining "state" of my combination and am backtracking where I should be (I think).

What am I doing wrong?

class Combo:
    def __init__(self, list):
       self.staples = list

Combo has a property called "staples", consisting of a list of staple classes. I want to iterate over the list of staples in a decision tree to find an optimal number.

In this case, the optimal number is summed across the quantities of each staple instance in the list and stored/recalculated as a property on the Combo instance.

def IterateStaples(combo, target):        
  #Exit condition for combo dictionary
  if all(combo.diff[macro] < 2 for macro in combo.diff):    
    return combo;                            

  #iterate through all items in list
  for staple in combo.staples:                                  

    #Increment and calc conditions
    staple.increment()         
    combo.calcTotals()      
    combo.findDiff(target)

    #If exceeds target value, backtrack
    if combo.findConflict(target):                
      staple.decrement()
      combo.calcTotals()                
      combo.findDiff(target)              

    #Redundant exit condition to try and return
    elif all(combo.diff[macro] < 2 for macro in combo.diff):                                  
      return combo                

    #Recursive call
    else:        
      return IterateStaples(combo, target)
      staple.decrement()
      combo.calcTotals()
      combo.findDiff(target)
Anthony Chung
  • 1,467
  • 2
  • 22
  • 44

3 Answers3

1

Your first if statement inside the for loop doesn't return anything. What it should return depends on your algorithm's logic:

#If exceeds target value, backtrack
if combo.findConflict(target):                
   staple.decrement()
   combo.calcTotals()                
   combo.findDiff(target)
   return SOMETHING

Additionally, the last 3 lines won't ever get executed, they're after the return statement.

Amit
  • 45,440
  • 9
  • 78
  • 110
  • Since I'm iterating over a collection and this branch of the decision tree is not useful to me, do I need to return something? I initially had a function call rather than a return in the last 4 lines. I am trying your suggestions now though. – Anthony Chung May 13 '16 at 19:49
  • "*this branch of the decision tree is not useful to me*" doesn't work for recursive algorithms. If an iteration relies on the result of a nested call, that nested call **must** return a relevant result. – Amit May 13 '16 at 19:51
  • @Amit Your answer makes the for loop useless, as it always exit the function at the first iteration – Valentin Lorentz May 14 '16 at 07:30
1

If I understand your code correctly (which is more difficult than usual, since you've not shown what most of the methods you're calling on combo and staple are), this should be what you want:

def IterateStaples(combo, target):        
    # base case
    if all(combo.diff[macro] < 2 for macro in combo.diff): # iterate on combo.diff.values()?
        return combo   # returning combo indicates success!

    for staple in combo.staples:
        staple.increment()                 # update state   
        combo.calcTotals()      
        combo.findDiff(target)

        if not combo.findConflict(target):  # skip recursing on invalid states
            result = IterateStaples(combo, target)    # recursive case
            if result is not None:      # if the recursion was successful, return the result
                return result

        staple.decrement()  # otherwise, undo the change to the state (backtrack)
        combo.calcTotals()     # these two lines might not be necessary when backtracking
        combo.findDiff(target) # since other branches will call them after staple.increment()

    return None # if we got to the end of the loop, explicitly return None to signal failure

The return None at the end is not strictly necessary, since None is the default return value if you don't return anything else. I just think that it's better to be explicit about it.

I'm following your code in returning combo on success (and extending it to returning None on failure). Since the code mutates combo in place, you could just as well return True for success (in the base case at the top of the function) and False for failure (at the bottom of the function, after the end of the loop). The recursive logic would pass on True results, and backtrack after False results. The top-level caller would need to check the combo instance they'd passed in for the actual solution if they got a True return value:

combo = Combo(something)
if IterateStaples(combo, target):
    do_stuff(combo) # success!
Blckknght
  • 100,903
  • 11
  • 120
  • 169
  • Hmm. I just tested it and it failed my unittests. It seems as though while it finds a solution, the mutated "combo" instance is not accurate. Will debug and keep you updated – Anthony Chung May 15 '16 at 14:09
  • Moved the base case to the bottom. Upon initialization, I immediately exited because I hadn't calculated diff (diff is initialized to 0) – Anthony Chung May 15 '16 at 14:16
  • I can't really help you with any of that stuff, since I have no idea what `calcTotals`, `findDiff` or the `increment` and `decrement` methods do. If you want a complete answer, you need to ask a complete question. – Blckknght May 15 '16 at 14:21
  • haha no problem. I have an issue now with the algo timing out. If I can't figure it out I'll ask a more complete question! :) – Anthony Chung May 15 '16 at 14:41
0

I was able to pass my own test case by incorporating a helper function in the following:

Is this not backtracking? I implemented N-queens with a similar approach

def IterateStaples(combo, target):        
  #iterate through all items in list
  bestCombo = []
  def helper(combo):
    for staple in combo.staples:                                      
      #Increment and calc conditions
      staple.increment()         
      combo.calcTotals()      
      combo.findDiff(target)    

      #If exceeds target value, backtrack
      if combo.findConflict(target):                      
        staple.decrement()
        combo.calcTotals()                
        combo.findDiff(target)                                        

      #Redundant exit condition to try and return
      elif all(combo.diff[macro] < 2 for macro in combo.diff):                                          
        bestCombo.append(deepcopy(combo))        
        return                

      #Recursive call
      else:        
        helper(combo)
        staple.decrement()
        combo.calcTotals()
        combo.findDiff(target)

  helper(combo)  

  return bestCombo
Anthony Chung
  • 1,467
  • 2
  • 22
  • 44