hffix icon indicating copy to clipboard operation
hffix copied to clipboard

hffix_fields.hpp: Use X macros to simplify and add msg_type enum

Open chris-pattern opened this issue 3 years ago • 2 comments

chris-pattern avatar Aug 12 '22 20:08 chris-pattern

oh i just read the comment that says to use the haskell script to regenerate. oof. in any case, using X macros will make it much easier for people to expand in the future / without having to edit this library manually. plus it would allow for actually editing the file instead of relying on scripts :)

chris-pattern avatar Aug 12 '22 20:08 chris-pattern

Thanks for the PR! I can't look at this right now; will look in a few weeks.

jamesdbrock avatar Aug 16 '22 06:08 jamesdbrock

@jamesdbrock bouncing this

chris-pattern avatar Apr 18 '23 18:04 chris-pattern

I appreciate the PR, but I don't really think that this PR is better. We don't want to enter all the FIX fields by hand, we want to auto-generate the C++ FIX field code from the Fields.xml file.

Further, we do want to make some manual adjustments to the generated code, and we can do that in the script, for example:

https://github.com/jamesdbrock/hffix/blob/8712946a02aedaf339b640e02d279bdeaadbf630/fixspec/spec-parse-fields/src/Main.hs#L60-L76

But I confess that figuring out how to adjust the generated code could be easier.

What adjustments to the generated hffix_fields.hpp file did you want to make? If you found some problems in the generated hffix_fields.hpp file and you want to fix them, then I'd like to do that.

jamesdbrock avatar Apr 19 '23 03:04 jamesdbrock

The functional changes is that I added a msg_type namespace that makes global constants for all the values in dictionary_init_message. It also massively reduces the code duplication.

In fact, you could make hffix_fields.hpp be a non-auto-generated file using X macros and then auto generate the table & have it be included.

autogenerated file hffix_fields_table.hpp would look like this:

#define HFFIX_FIX_FIELDS(X) \
X(Account,                                                               1) /*!< 1 (String FIX.2.7) Account mnemonic as agreed between buy and sell sides, e.g. broker and institution or investor/intermediary and fund manager.*/ \
X(AdvId,                                                                 2) /*!< 2 (String FIX.2.7) Unique identifier of advertisement message. \
 \
(Prior to FIX 4.1 this field was of type int)*/ \
...

chris-pattern avatar Apr 19 '23 16:04 chris-pattern

So this is what you think the definitions should look like (with the X macro HFFIX_FIX_MSGTYPE)?

namespace msg_type {
#if __cplusplus >= 201103L
#define HFFIX_dummy(name, value) static constexpr const char* const name = value;
#else
#define HFFIX_dummy(name, value) static const char* const name = value;
#endif
HFFIX_FIX_MSGTYPE(HFFIX_dummy)
#undef HFFIX_dummy
} // namespace msg_type

This looks good to me.

jamesdbrock avatar Apr 23 '23 13:04 jamesdbrock

Thank you very much for this PR @chris-pattern . I used your code for adding a hffix::msg_type namespace in #47 . I'm not going to use your PR because I don't want to add an extra “X macros” indirection.

jamesdbrock avatar Apr 28 '23 15:04 jamesdbrock

Cool!

chris-pattern avatar Apr 28 '23 15:04 chris-pattern

Ok so looping back to this -- the benefit of using X macros is that I can then expand your functions or reuse the enum in a different way than you programmed up in this module. For example, you don't support tag 1362 https://www.onixs.biz/fix-dictionary/5.0.sp2/tagnum_1362.html . I'm working with some exchanges that define their own tags too :(

chris-pattern avatar May 05 '23 15:05 chris-pattern

you don't support tag 1362 https://www.onixs.biz/fix-dictionary/5.0.sp2/tagnum_1362.html

Remember that you can always just use the tag number 1362 because tags are ints in hffix.

You can name the tag if you want

const int nofills = 1362;

You can add also add it to your field dictionary

std::map<int, std::string> fields;
hffix::dictionary_init_field(fields);
fields[1362] = "NoFills";

and you can read field 1362 as_int()

jamesdbrock avatar May 06 '23 14:05 jamesdbrock