duckduckgo / zeroclickinfo-goodies

DuckDuckGo Instant Answers based on Perl & JavaScript
https://duckduckhack.com/
Other
979 stars 1.76k forks source link

Fibonacci: Use perl package for generating sequence #3068

Closed GuiltyDolphin closed 8 years ago

GuiltyDolphin commented 8 years ago

Instead of rolling our own, we could use a package like Math::NumSeq::Fibonacci to generate the Fibonacci numbers.


IA Page: http://duck.co/ia/view/fibonacci Maintainer: @koosha--

koosha-- commented 8 years ago

The current code is working fine and is concise and I don't see any point in this switch.

mintsoft commented 8 years ago

@GuiltyDolphin Is there any specific benefit to switching? (Performance perhaps?)

GuiltyDolphin commented 8 years ago

@mintsoft The package is faster, has more features (if we wish to expand) and more robust; is it not one of the Perl sayings "Don't re-invent the wheel"?

Anyway, some cool things this package provides:

Also there's a bunch of other sequences in Math::NumSeq so if this were a dependency it would open those up to others.

mintsoft commented 8 years ago

Well that's cool; if we want to extend the functionality of the IA then I agree it makes sense to switch it out at that time

koosha-- commented 8 years ago

Unless you have a concrete utilization of its "more features" for extending, I would still not see any particular point in switching and yet having the same functionality as we do now. In other words, the switch should be made only if you have added a new feature to the code that comes from the proposed module.

GuiltyDolphin commented 8 years ago

@koosha-- I quite like the idea of being able to ask is X a fibonacci number - I'll make an issue for it.

sentientcucumber commented 8 years ago

I was going to start with this issue before adding the functionality in #3070 and had a couple of questions.

  1. Based on the context, it seems like the current implementation limits requests to 1470 because of precision. Should we still impose the limit or should I play around with it until performance starts to degrade?
  2. I noticed its using remainder, rather than remainder_lc. Thus, "What is the 5th Fibonacci?" doesn't trigger, where "what is the 5th fibonacci" does. Is that desired? Also, for my own information, it seems like both "Fibonacci" and "fibonacci" trigger, assuming triggers are not case sensitive?

Thanks!

EDIT: Played around with it using duckpan query, I don't notice a time difference between querying for the 1470th or the 25000th number, using the Perl package. Tested with even higher numbers with out much delay (until you start hitting over 100000), but kind of question how useful that is.

GuiltyDolphin commented 8 years ago

@shellhead Thanks.

Yeah, generally we want case-insensitive triggering (unless there is a good reason for it :) ).

Yah, the Fib package is a lot faster so you wont notice much difference until really high queries - I'd imagine a sensible limit would be fine.