Skip to content
This repository has been archived by the owner on Jun 22, 2020. It is now read-only.

Add Order class to abstract formatting into [price, amount, timestamp] #175

Merged
merged 1 commit into from
Oct 20, 2017

Conversation

tmlee
Copy link
Member

@tmlee tmlee commented Oct 18, 2017

  • What is the purpose of this Pull Request?
    When implementing OrderBook, there is no structure for individual Order.
    Here is an abstraction of a Class, when assigned with price, amount, and timestamp will spit out the format we expect for all exchanges

@timestamp = args[:timestamp] || nil
end

def format_as_list
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tmlee Actually, why do we need to do this? Can't we just return an array of Cryptoexchange::Models::Order ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dominicwong617 That is possible. We just need to decide what end result we want to provide. Though i think returning as Order would give the user best flexibility how they want to store the results.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dominicwong617 now giving the user the choice to call this if they want to

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tmlee let's just return an array of object as asks and bids, this choice doesn't add much benefits

@tmlee tmlee merged commit 5990d51 into coingecko:master Oct 20, 2017
@tmlee tmlee mentioned this pull request Oct 20, 2017
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants