0
\$\begingroup\$

Does anyone have any ideas on how I can refactor these methods to get rid of the duplication? The first, third and fourth lines of both methods do the same thing. I'm having trouble for some reason because the second argument of both methods are of different types (or rather they represent different types of values).

I was think of using procs and having the different code be a block, but I couldn't find a solution.

def buy_in_cash(symbol, cash_amt) # second argument is cash
  market_price = @exchange.price_by_symbol(symbol)
  coin_amt = cash_to_coin(cash_amt, market_price) # different
  withdraw_cash(cash_amt)
  @transactions << Transaction.new(symbol, coin_amt, market_price)
end

def buy_in_coin(symbol, coin_amt) # here second argument is coin
  market_price = @exchange.price_by_symbol(symbol)
  cash_amt = coin_to_cash(coin_amt, market_price) # different
  withdraw_cash(cash_amt)
  @transactions << Transaction.new(symbol, coin_amt, market_price)
end
\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$

In my opinion, "buying" should be encapsulated in it's own method that needs to know the symbol, cash and coin amounts.

Since buying is transactional and both actions need to happen or be rolled back, I think it's a good candidate for it's own method and shouldn't be mixed with converting currencies whioch in my opinion is a pre-requisite to buying. If, for example, there is an error with converting the currencies, buy shouldn't be called at all.

Moreover, there may be side effects to buying like managing stock and dispatching the goods so that makes it an even better candidate for it's own method.

Maybe like this:

def buy(symbol, cash_amt, coin_amt)
  # Both coin and cash amount need to be passed
  raise ArgumentError unless coin_amt && cash_amt

  withdraw_cash(cash_amt)
  @transactions << Transaction.new(symbol, coin_amt, market_price)
end

def market_price(symbol)
  @exchange.price_by_symbol(symbol)
end

def buy_in_cash(symbol, cash_amt)
  coin_amt = cash_to_coin(cash_amt, market_price(symbol))
  buy(symbol, cash_amt, coin_amt)
end

def buy_in_coin(symbol, coin_amt)
  cash_amt = coin_to_cash(coin_amt, market_price(symbol))
  buy(symbol, cash_amt, coin_amt)
end
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.