ollieglass / share-counter

Check how many times a page/URL has been shared on social networks and aggregators
MIT License
12 stars 6 forks source link

returns nil instead of 0 #11

Open bricemaurin opened 9 years ago

bricemaurin commented 9 years ago

when no results on stumbleupon (other networks too), it will return nil instead of 0.

to fix that, you could probably have a private method that checks if the json is empty and return 0 if so. What do you think?

bricemaurin commented 9 years ago

I used this in my code:

# handle nil value returned by stumbleupon API
@counts[:stumbleupon] = ShareCounter.stumbleupon @url
@counts[:stumbleupon] = 0 if @counts[:stumbleupon] == nil

# return 0 when delicious API fails
@counts[:delicious] = ShareCounter.delicious @url
@counts[:delicious] = 0 if @counts[:delicious] == -1
bboroda commented 9 years ago

Didn't realize folks still use delicious (nothing wrong with that, was just surprised)

tryzniak commented 9 years ago

Hmmm, how about allowing a user to specify custom behavior when ShareCounter can't fetch information? Something like:

ShareCounter.all { 0 } # will return 0s for each failed network and actual values for available networks , i.e { facebook: 0, twitter: 0, delicious: 10 }

ShareCounter.all { raise StandardError } # will raise StandardError and stop fetching

ShareCounter.all { |failed_nw| puts "#{failed_nw} didn't respond"; 0 } # and we might also yield the name of the failed network to the block

ShareCounter.facebook { nil } # same applies for each network

ShareCounter.all # and there would be a standard behavior for the case when a block is not given, for example we might raise an exception.
ollieglass commented 9 years ago

Interesting...

Alternatively, how about two versions of the method, a bang version that returns raw errors from the provider, and the normal one that returns nil when there's an error?

Sharecounter.all
Sharecounter.all!
Sharecounter.facebook
Sharecounter.facebook!
tryzniak commented 9 years ago

@ollieglass, good idea! And the normal version would also allow us to pass in a block?

ollieglass commented 9 years ago

I'm not sure what the pros and cons are of being able to pass a block to the method - could you outline them?

tryzniak commented 9 years ago

Well, I believe it could make the API a bit more flexible. For example a user calls 'all' method and then wants to summarize counts into a variable, like so:

shares = ShareCounter.all
sum = shares.values.reduce { |acc, sc| acc + sc.to_i } #example 1
#or
sum = shares.values.reduce { |acc, sc| acc + (sc.nil? ? 0 : sc) } #example 2. same thing as example 1

but instead with custom blocks they might've done:

shares = ShareCounter.all { 0 } # all nils are 0s now, because we are handing errors with the block, sort of substituting errors with the thing that the block returns
sum = shares.values.reduce(&:+) # now we don't need to check for nils

And the blocks would be optional. No need to force using them.

Or for example a user wants to stop fetching counts immediately if an error occurs, in addition, they want to raise a custom exception in that case:

ShareCounter.all { raise BigBuddaBoom }
ollieglass commented 9 years ago

That looks good, let's go with blocks! Would you like to make a pull request?

tryzniak commented 9 years ago

I haven't started coding it yet :smiley: But I think it won't take long, so stay tuned :+1: