0

I have the following method:

def all(response = nil, options = {}, data = [])
  response ||= get(resource_path, options)
  parsed_response = JSON.parse(response.body)
  return bad_response(response) unless response.code == '200'

  data += parsed_response['data'].nil? ? [] : parsed_response['data'].map { |obj| new(obj) }
  puts data.count
  if parsed_response.dig('additional_data', 'pagination', 'more_items_in_collection')

    options[:start] = parsed_response.dig('additional_data', 'pagination', 'next_start')
    all(nil, options, data) # recursively calls itself, passing cumulated data
  end
  puts data.count # WHY WOULD IT NOT JUST RETURN ONCE, BUT INSTEAD GO DOWN TO INITIAL VALUE?
  data
end

My problem with this method, is that when it's time to exit the method: (if parsed_response.dig('additional_data', 'pagination', 'more_items_in_collection') returns false and the last method's line should be fired), it starts to rewind..

The output is:

Pipedrive::Customer.all.count
100
200
300
400
500
600
700
800
900
1000
1100
1200
1300
1400
1500
1600
1700
1800
1900
2000
2100
2200
2300
2400
2500
2600
2700
2800
2900
3000
3100
3200
3300
3400
3500
3600
3700
3800
3900
3991
3991
3900
3800
3700
3600
3500
3400
3300
3200
3100
3000
2900
2800
2700
2600
2500
2400
2300
2200
2100
2000
1900
1800
1700
1600
1500
1400
1300
1200
1100
1000
900
800
700
600
500
400
300
200
100
=> 100

This looks insane, but I hope I'm just missing something extremely simple (indeed I am, but what exactly :) ).

EDIT

My problem is with the result of the method, not with puts itself. I would expect the Pipedrive::Customer.all.count to return 3991, but it returns 100 instead... Any ideas? I already tried to put last line (data) in else

  def all(response = nil, options = {}, data = [])
    response ||= get(resource_path, options)
    parsed_response = JSON.parse(response.body)
    return bad_response(response) unless response.code == '200'

    data += parsed_response['data'].nil? ? [] : parsed_response['data'].map { |obj| new(obj) }
    if parsed_response.dig('additional_data', 'pagination', 'more_items_in_collection')

      options[:start] = parsed_response.dig('additional_data', 'pagination', 'next_start')
      all(nil, options, data)
    else
      data
    end
    data 
  end

- it still returns 100..

Andrey Deineko
  • 51,333
  • 10
  • 112
  • 145
  • I don't see a base case here... – Anthony Apr 13 '16 at 20:12
  • 2
    @Anthony the base case looks to be when there aren't any more items in the collection, in which case `parsed_response.dig(...)` will return `false`; I think Andrey meant to put the `puts data.count; data` in an `else` block – Ken Bellows Apr 13 '16 at 20:15
  • 1
    So after the edit `all` now returns `data`, but you don't do anything with the return value from your recursive call. In other words, you return `data` but then drop it on the kitchen floor. – pjs Apr 13 '16 at 20:27
  • got it working, finally! thx @pjs – Andrey Deineko Apr 13 '16 at 20:32
  • If all you want is the updated `data`, take out `data` right before the final `end` statement. With the if-else, you'll either get the `data` returned by the recursive call to `all`, or the `data` handled by the `else` case. Otherwise, save the return from `all` and do something with it before leaving the current invocation. – pjs Apr 13 '16 at 20:32
  • You're welcome! Glad to hear it's working now. – pjs Apr 13 '16 at 20:43

2 Answers2

2

You only need and want one puts statement in the method. Right now you're telling it to puts data.count, then do recursive calculations, and after returning from those recursive calculations puts data.count again. In between those two puts, the recursive call(s) will have injected their own puts results coming and going. That's why it appears to build up, then unravel. The build-up is the puts's going in, and the rewind is from the puts's after the recursive call.

pjs
  • 18,696
  • 4
  • 27
  • 56
  • thx for reply! my main problem not in puts (puts is only to try tracing), - the issue is that method returns 100 instead of 3991 (as I do `Pipedrive::Customer.all.count`).. – Andrey Deineko Apr 13 '16 at 20:20
  • 1
    @AndreyDeineko That's nowhere to be found in your question, you only discussed the unraveling as illustrated by the printed output. Clarifying your question would appear to be in order. – pjs Apr 13 '16 at 20:22
  • my bad, sorry for not being clear - edited the question! – Andrey Deineko Apr 13 '16 at 20:24
2

The problem is that you're not exiting the function after your recursive call, so when it returns from the recursive call it goes on and finished the function, thus calling puts data.count a second time for each recursive call on its way out. The reversed order has to do with the function stack. your code is structured something like this:

all:
  puts 100
  all:
    puts 200
    all:
      puts 300
      all:
        ...
      puts 300
    puts 200
  puts 100
done

Based on your code I think you meant to put the second puts and the data in an else maybe? that should do what you want I think, though I don't fully grok your code

Ken Bellows
  • 6,711
  • 13
  • 50
  • 78