CODEWITHSUNDEEP
ruby
OG Newbie
OG Newbie

Reputation: 45

I'm trying to do a stock picker method on Ruby but i have some issue in my code

I'm trying to do a stock picker method that takes in an array of stock prices, one for each hypothetical day. It should return a pair of days representing the best day to buy and the best day to sell. Days start at 0.

def stock_picker stocks
  pair = []

  if stocks.size < 2
    return "Please enter an array with a valid number of stocks"
  else
    buy_day = 0
    sell_day = 0
    profit = 0

    stocks.each_with_index do |buy, index|
      i = index
      while (i < stocks[index..-1].size)
        if ((buy - stocks[i]) > profit)
          profit = buy - stocks[i]
          buy_day = stocks.index(buy)
          sell_day = i
        end
        i+= 1
      end

    end
    pair = [buy_day,sell_day]
    return pair.inspect
  end
end

stock_picker([17,3,6,9,15,8,6,1,10])

It should return [1,4] instead of [0,7]

Upvotes: 1

Views: 717

Answers (3)

PhiAgent
PhiAgent

Reputation: 158

You can loop through the stock_prices array selecting for days with greatest positive difference. Your while condition needs to be changed.

#steps
#sets value of biggest_profit to 0(biggest_loss if looking for loss)
#sets most_profitable_days to [nil,nil]
#loops through array
#takes buy day
#loops through remainder of array
#if current day-first day>biggest_profit (first_day-current_day for loss)
#make >= for shortest holding period
#reassign biggest_profit
#most_profitable_days.first=buy_day, most_profitable_days.last=sell_day
#sell_day & buy_day are values of indices

#tests
#must accept only array
#must return array
#must return correct array

def stock_picker(arr)

    #checks to make sure array inputs only are given
    raise 'Only arrays allowed' unless arr.instance_of?(Array)

    #sets value of biggest_profit to 0(biggest_loss if looking for loss)
    biggest_profit=0

    #sets most_profitable_days to [nil,nil]
    most_profitable_days=[nil,nil]

    #loops through array
    arr.each_with_index do |starting_price, buy_day|

        #takes buy day
        arr.each_with_index do |final_price,sell_day|

            #loops through remainder of array
            next if sell_day<=buy_day

            #if current day-first day>biggest_profit (first_day-current_day for loss)
            #make '>=' for shortest holding period
            if final_price-starting_price>=biggest_profit

              #reassign biggest_profit
              biggest_profit=final_price-starting_price

              #most_profitable_days.first=buy_day, 
              most_profitable_days[0]=buy_day#+1 #to make it more user friendly

              #most_profitable_days.last=sell_day
              most_profitable_days[-1]=sell_day#+1 #to make it more user friendly
            end
        end
    end
    
  #return most_profitable_days
  most_profitable_days
end
p stock_picker([3,2,5,4,12,3]) #[1,4]

Upvotes: 1

iGian
iGian

Reputation: 11193

Another option is to slice the Array while iterating over it for finding the best profit:

res = ary.each_with_index.with_object([]) do |(buy_val, i), res|
  highest_val = ary[i..].max
  highest_idx = ary[i..].each_with_index.max[1] + i
  res << [highest_val - buy_val, i, highest_idx]
end.max_by(&:first)

#=> [12, 1, 4]

Where 12 is the profit, 1 is the buy index and 4 is the sell index.

To understand how it works, run this extended version, it worth more than any written explanation:

res = []
ary.each_with_index do |buy_val, i|
  p buy_val
  p ary[i..]
  p highest_val = ary[i..].max
  p highest_idx = ary[i..].each_with_index.max[1] + i
  res << [highest_val - buy_val, i, highest_idx]
  p '----'
end

res #=> [[0, 0, 0], [12, 1, 4], [9, 2, 4], [6, 3, 4], [0, 4, 4], [2, 5, 8], [4, 6, 8], [9, 7, 8], [0, 8, 8]]

From the Ruby standard library I used Enumerable#each_with_index, Enumerable#each_with_object, Enumerable#max and Enumerable#max_by.

For getting the index of the max I kindly stole from Chuck (https://stackoverflow.com/a/2149874), thanks and +1. I didn't look for any better option.

As per a comment from Cary Swoveland in the linked post:

[..] a.index(a.max) will return the index of the first and a.each_with_index.max[1] will return the index of the last [..]

So, maybe you want to use the first option to keep the time between buy and sell shorter.

Upvotes: 1

Aleksei Matiushkin
Aleksei Matiushkin

Reputation: 121010

Use Array#combination:

stocks.
    each_with_index.
    to_a.
    combination(2).
    select { |(_, idx1), (_, idx2)| idx2 > idx1 }.
    reduce([-1, [-1, -1]]) do |(val, acc), ((v1, idx1), (v2, idx2))|
  val < v2 - v1 ? [v2 - v1, [idx1, idx2]] : [val, acc]
end
#⇒ [ 12, [1, 4] ]

Upvotes: 1

Related Questions