herval / yahoo-finance

[DEPRECATED] A simple wrapper for yahoo finance quotes end-point.
192 stars 72 forks source link

Invalid columns_array hash key passed to quotes method causes incorrect results #35

Closed bcoles closed 7 years ago

bcoles commented 7 years ago

Passing an invalid columns_array hash key to the quotes method causes incorrect results in the values for the following keys. The final n key values are nil based on n invalid keys supplied.

Example - Valid keys

#!/usr/bin/env ruby
require 'yahoo-finance'

yahoo_client = YahooFinance::Client.new
data = yahoo_client.quotes(
  ["TLS.AX"],
  [:name,
   :shares_outstanding,
   :float_shares,
   :ebitda,
   :market_capitalization,
   :pe_ratio,
   :low_52_weeks,
   :high_52_weeks,
   :last_trade_price])

puts data.inspect

Result:

[#<OpenStruct name="TELSTRA FPO", shares_outstanding="12206597000", float_shares="12118221000", ebitda="9.54B", market_capitalization="59.81B", pe_ratio="10.36", low_52_weeks="4.89", high_52_weeks="5.86", last_trade_price="4.90">]

Example - Invalid key doesnotexist:

#!/usr/bin/env ruby
require 'yahoo-finance'

yahoo_client = YahooFinance::Client.new
data = yahoo_client.quotes(
  ["TLS.AX"],
  [:name,
   :doesnotexist,
   :shares_outstanding,
   :float_shares,
   :ebitda,
   :market_capitalization,
   :pe_ratio,
   :low_52_weeks,
   :high_52_weeks,
   :last_trade_price])

puts data.inspect

Result:

[#<OpenStruct name="TELSTRA FPO", doesnotexist="12206597000", shares_outstanding="12118221000", float_shares="9.54B", ebitda="59.81B", market_capitalization="10.36", pe_ratio="4.89", low_52_weeks="5.86", high_52_weeks="4.90", last_trade_price=nil>]

Note that the doesnotexist key is present in the result and the value for shares_outstanding has moved to the doesnotexist key. The value for the final key last_trade_price is nil.

Patch

The following is a possible patch which returns nil from the quotes method in the event an invalid key is supplied. A better approach would be to drop the invalid key from columns_array before performing the query, or possibly raise an error instead.

diff --git a/lib/yahoo-finance.rb b/lib/yahoo-finance.rb
index ef924da..3e6c4f6 100755
--- a/lib/yahoo-finance.rb
+++ b/lib/yahoo-finance.rb
@@ -115,6 +115,13 @@ module YahooFinance
       :symbol, :last_trade_price, :last_trade_date,
       :change, :previous_close], options = {})

+      columns_array.each do |c|
+        unless COLUMNS.key?(c)
+          # TODO: raise an error
+          return nil
+        end
+      end
+
       options[:raw] ||= true
       ret = []
       symbols_array.each_slice(SYMBOLS_PER_REQUEST) do |symbols|

Edit: In hindsight it might be easier just to drop the keys. I've submitted PR #36 for this.