Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some comments as I attempt to understand what's going on #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

some comments as I attempt to understand what's going on #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba5d723
some comments as I attempt to understand what's going on
JonathanQuang Dec 17, 2019
440b46b
doxygen autogeneration done with command: make doc
JonathanQuang Dec 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
CXX := g++
CXXFLAGS := -std=c++2a -O3 -Wall -Wpedantic -MMD
CXXFLAGS := -std=c++17 -O3 -Wall -Wpedantic -MMD
Copy link
Contributor

Choose a reason for hiding this comment

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

obv discussion we had about this before, but i'll enumerate here so it's in writing on the repo:

C++17 makes some stuff not possible rn, i.e. we can't do struct initialization like {.x = 5}. Easy fix, we can just use constructors until this gets added in C++20. Might be useful to make a quick PR to get that & we can merge that before this PR just so we don't get warnings when we merge this. Happy to take care of that myself

-include $(OBJ_FILES:.o=.d)

SOURCES := $(wildcard src/*.cc)
Expand All @@ -21,3 +21,6 @@ $(TARGET): $(OBJECTS)
build/%.o: src/%.cc
$(CXX) $(CXXFLAGS) -c -o $@ $<

docs:
@doxygen doxy.cfg

2,494 changes: 2,494 additions & 0 deletions doxy.cfg
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions src/agent.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@

namespace tagt::xchg::agent {

/**
* Prints out XCHG=> followed by whatever string data is associated with agent ID (uint_16)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this comment is one of those comments that talks too much about implementation rather than purpose. Right now this is temporary behavior and in the future agent::reply is going to send a message to the agent over TCP, so I think the documentation should reflect that. Something like:

/**
 * Sends a reply to a client (agent)
 * @param agent The ID for the client to contact
 * @param response The packet to send to the client

* and the response
* @param agent something to do with the ID of the agent
* @param response something about a packet struct
*/
void reply(Id agent, const packet::Packet& response) {
std::cout << "[XCHG=>" << agent << "]: " << response.serialize() << std::endl;
}
Expand Down
12 changes: 12 additions & 0 deletions src/book.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ template<order::Direction>
inline std::shared_ptr<const order::Order> best
(std::shared_ptr<const order::Order> a, std::shared_ptr<const order::Order> b);

/**
* @param a shared ptr to an order
* @param b shared ptr to another order
* @return shared ptr to order with lowest price we can buy something
*/
template<>
inline std::shared_ptr<const order::Order> best<order::Direction::BUY>
(std::shared_ptr<const order::Order> a, std::shared_ptr<const order::Order> b) {
Expand All @@ -16,6 +21,13 @@ inline std::shared_ptr<const order::Order> best<order::Direction::BUY>
}
}

/**
* Returns shart ptr to the order that has the highest range.upper value for
Copy link
Contributor

Choose a reason for hiding this comment

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

shart ptr

* the highest value we can sell something
* @param a shared ptr to an order
* @param b shared ptr to another order
* @return shared ptr to order that has the highest range.upper val for selling
*/
template<>
inline std::shared_ptr<const order::Order> best<order::Direction::SELL>
(std::shared_ptr<const order::Order> a, std::shared_ptr<const order::Order> b) {
Expand Down
4 changes: 2 additions & 2 deletions src/book.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ namespace tagt::xchg::book {
struct Book {
size_t next_order_id { 0 };
std::vector<order::Id> order_ids;
std::unordered_map<order::Id, std::shared_ptr<order::Order>> orders;
std::unordered_map<order::Ticker, order::Price> last_price;
std::unordered_map<order::Id, std::shared_ptr<order::Order>> orders;/**maps Id to shared_ptr*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably more useful that this is mapping to the Order rather than the shared_ptr, no?

std::unordered_map<order::Ticker, order::Price> last_price;/**maps Ticker to Price */
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, don't mean to get repetitive but I feel like this is one of those comments that doesn't really tell you anything new other than looking at the code. I'd like to explain the why here rather than the what. I.e. why am I tracking the last price? How does the code take care of that? Why are we using these data structures? etc.


template <class OrderType>
auto match(OrderType& incoming) ->
Expand Down
27 changes: 27 additions & 0 deletions src/order.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,34 @@

namespace tagt::xchg::order {

/**
* @return whether this range's lower is actually lower than upper bound
*/
bool Range::is_valid() const {
return this->lower <= this->upper;
}

/**
* @param other pointer to another Range object
* @return a Range object with the highest of the .lower and smallest of the .upper
*/
Range Range::overlap(const Range& other) const {
return {
.lower = std::max(this->lower, other.lower),
.upper = std::min(this->upper, other.upper)
};
}

/**
* @return constant Range with the smallest possible price and largest possible price
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: Why does it have that behavior?
A: Something to do with how market orders are infinitely elastic (is that the right word?) to price

*/
Range Market::range() const {
return { .lower = PRICE_MIN, .upper = PRICE_MAX };
}

/**
* @return a constant range with this order's price as the upper bound for a buy or lowerbound for a sell
*/
Range Limit::range() const {
switch (this->direction) {
case Direction::BUY:
Expand All @@ -28,6 +41,9 @@ Range Limit::range() const {
}
}

/**
* @return a constant range with this order's price as the lower bound for a buy or upper bound for a sell
*/
Range StopLoss::range() const {
switch (this->direction) {
case Direction::BUY:
Expand All @@ -38,6 +54,9 @@ Range StopLoss::range() const {
}
}

/**
* @return a constant range with buy:[stop_price, limit_price] or sell[limit,stop]
*/
Range StopLimit::range() const {
switch (this->direction) {
case Direction::BUY:
Expand All @@ -48,6 +67,10 @@ Range StopLimit::range() const {
}
}

/**
* @param other pointer to another order
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, what is the point of this method? A: Check to see whether two orders "match", i.e. can they trade against each other?

* @return true if tickers match, directions are different, range overlaps valid, (no AON flag or this size smaller than other)
*/
bool Order::match(const Order& other) const {
switch (this->direction) {
case Direction::BUY:
Expand All @@ -64,6 +87,10 @@ bool Order::match(const Order& other) const {
}
}

/**
* @param other pointer to another order
* @return Price of lower of overlap if buy, upper of overlap if sell
*/
Price Order::get_match_price(const Order& other) const {
auto my_range = this->range();
auto their_range = other.range();
Expand Down
4 changes: 2 additions & 2 deletions src/order.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ using Price = uint32_t;
enum class Direction { BUY, SELL };

struct Flags {
bool AON {false}; // All-or-none
bool IOC {false}; // Immediate-or-cancel
bool AON {false}; /* All-or-none */
bool IOC {false}; /* Immediate-or-cancel */
};

struct Range {
Expand Down