jamesdbrock / hffix

Financial Information Exchange Protocol C++ Library
http://jamesdbrock.github.io/hffix
Other
276 stars 89 forks source link

buffer-management #26

Open jamesdbrock opened 5 years ago

jamesdbrock commented 5 years ago

Split out the buffer-management features of message_reader and message_writer into separate classes?

The issue I guess comes down to

for (; reader.is_complete(); reader = reader.next_message_reader()) {

and

hffix::message_writer new_order(logon.message_end(), buffer + sizeof(buffer));

being kind of weird.

jamesdbrock commented 5 years ago

(These lines are from the example programs writer01.cpp and reader01.cpp.)

On reflection, this is fine:

for (; reader.is_complete(); reader = reader.next_message_reader()) {

But maybe this

hffix::message_writer new_order(logon.message_end(), buffer + sizeof(buffer));

should be more like:

hffix::message_writer new_order = logon.next_message_writer();

Though the current system is a pretty simple API and if we are going to improve it, we want to make sure that the improvements are even simpler.

next_message_writer() method should explain that it returns a new message_writer constructed on the same buffer, but beginning where the current message ended.

We cannot, in general, ensure that when a message_writer is constructed that it will have enough space to write the message, so we shouldn't try to do that.

The problem with this whole thing is that is has the effect of hiding the buffer arithmetic instead of making the buffer arithmetic explicit and transparent. If we append messages to the buffer with next_message_writer(), then when it's time to do I/O we go back to explicit buffer arithmetic with

std::cout.write(buffer, new_order.message_end() - buffer);
dingobye commented 5 years ago

I think the current design is alright.

It makes sense for users to explicitly mange the buffer arithmetics, since message_writer is designed not to own any buffer, but only works on a given chunk of contiguous memory (with begin_ and end_). It is the user's responsibility to organise the offsets of one buffer shared by multiple message_writer instances, who are lack of a global view as individuals.

jamesdbrock commented 4 years ago

I suspect that reader = reader.next_message_reader is causing #27 . What we really need is a whole message_reader_buffer abstraction with iterators which dereference to message_readers. That would be a massive breaking API change though.