hffix icon indicating copy to clipboard operation
hffix copied to clipboard

Fix next_message_reader() method

Open Roman-Koshelev opened this issue 4 years ago • 4 comments

According to the comment, the function must return a valid message (although not necessarily complete). This is logical, but not so now. Also, now, in the absence of a valid message, we will return a zero-size reader pointing to the last byte

Roman-Koshelev avatar Apr 11 '21 10:04 Roman-Koshelev

Please explain further? How is next_message_reader supposed to behave after this PR?

Honestly, this whole business with automatically searching for the next 8=FIX string after an invalid message was probably a bad idea to begin with. Today I would not recommend that anyone use that feature. If a user encounters some invalid FIX, my advice would be to stop parsing, stop trading, close the connection, and get on the phone immediately with the person who sent the invalid FIX.

Users can write their own function to scan through invalid garbage bytes looking for some valid FIX, if they really want to. Which might be a good idea sometimes, like when the user is trying to parse and audit old trading logs. But choosing which kinds of garbage are allowable is very specific to the situation, and this library shouldn't guess about that.

jamesdbrock avatar Apr 17 '21 09:04 jamesdbrock

I agree with you, but I think that this will have to stay at least for the sake of backward compatibility.

  1. In the current implementation, we are simply looking for "8=FIX". This contradicts " if there might be a complete or partial valid message anywhere else in the remainder of the buffer, will return a new message_reader constructed at that location.". I think if we find the first occurrence at the possible beginning of the FIX message and it is not the beginning, then we must continue the search.
  2. If there is no partial or complete valid message in the rest of the buffer, we must return an empty buffer pointing to the last byte. In the current implementation, we return the reader to the last 10 bytes. What does it mean? what does begin () of this buffer point to? Unclear...

Roman-Koshelev avatar Apr 17 '21 11:04 Roman-Koshelev

If there is no valid message in the rest of the buffer, then we don't know where the last byte is.

That's why the message_reader::end() says

std::logic_error if called on an invalid message. This exception is preventable by program logic. You should always check if a message is_valid() before reading.

https://jamesdbrock.github.io/hffix/classhffix_1_1message__reader.html#a766234db0fadea164a1e4f71f49af381

This is awkward, but it's pretty common practice in C++. It would be better if we could make this invalid state unrepresentable so that the compiler could verify that we never call end() on an invalid message, but I don't know how to do that in a language like C++ which has no algebraic sum types.

jamesdbrock avatar May 09 '21 07:05 jamesdbrock

message_begin and prefix_* does not throw an exception. I still find the change very useful

Roman-Koshelev avatar May 09 '21 07:05 Roman-Koshelev