Skip to content

Conversation

the-thing
Copy link
Contributor

Hi @chrjohn,

Custom fields and groups are added to parent now (ie. setting, getting and copying is supported). Please keep in mind that when support of custom fields and groups is required new methods must be used. Previous methods:

  • quickfix.FieldMap#setComponent(MessageComponent component)
  • quickfix.FieldMap#getComponent(MessageComponent component)
  • quickfix.MessageComponent#copyFrom(FieldMap fields)
  • quickfix.MessageComponent#copyTo(FieldMap fields)

are backwards compatible - we don't want to break existing behaviour on production + there is an extra array creation required to copy all or selected fields/groups.

There are new methods that allow copying all fields and groups:

  • quickfix.FieldMap#getComponent(MessageComponent component, boolean includeAllFields, boolean includeAllGroups)
  • quickfix.FieldMap#getComponent(MessageComponent component, int[] includeFields, int[] includeGroups)
  • quickfix.FieldMap#setComponent(MessageComponent component, boolean includeAllFields, boolean includeAllGroups)
  • quickfix.FieldMap#setComponent(MessageComponent component, int[] includeFields, int[] includeGroups)
  • quickfix.MessageComponent#copyFrom(FieldMap fields, boolean includeAllFields, boolean includeAllGroups)
  • quickfix.MessageComponent#copyFrom(FieldMap fields, boolean includeAllFields, boolean includeAllGroups)
  • quickfix.MessageComponent#copyTo(FieldMap fields, boolean includeAllFields, boolean includeAllGroups)
  • quickfix.MessageComponent#copyTo(FieldMap fields, boolean includeAllFields, boolean includeAllGroups)

Also new methods are generated for each component, message and fix dictionary:

  • #set(XXX component, boolean includeAllFields, boolean includeAllGroups)
  • #set(XXX component, int[] includeFields, int[] includeGroups)
  • #get(XXX component, boolean includeAllFields, boolean includeAllGroups)
  • #get(XXX component, int[] includeFields, int[] includeGroups)
  • #getXXX(boolean includeAllFields, boolean includeAllGroups)
  • #getXXX(int[] includeFields, int[] includeGroups)

It seems that quickfixj-all.jar file is 1.2MB larger (size 22.6 MB, compared to quickfixj-all-2.1.0.jar which is 21.4 MB)

@the-thing the-thing changed the title QFJ-636 QFJ-636 - Custom fields in component blocks are not added to parent Aug 26, 2018
@chrjohn
Copy link
Member

chrjohn commented Aug 28, 2018

Hi @the-thing ,

thanks for the PR. Will have a look at it later.
W.r.t. the message size: you mean that after your changes the JAR file size increases due to the newly generated methods in every message class?

Thanks,
Chris.

@the-thing
Copy link
Contributor Author

Hi @chrjohn,

The FIX message size will not change. I made a note that about increased jar file sizes. I'm still wondering what is your opinion on creating new methods against only changing the existing ones.

Thanks,
Marcin

@chrjohn
Copy link
Member

chrjohn commented Aug 28, 2018

Hi @the-thing ,
I was referring to the generated message classes, not the FIX messages on the wire. :)
Hmm, if I understood correctly the current behaviour (i.e. not copying everything from the message) is a bug. So there probably is nothing wrong with changing the existing methods to copy everything. However, the extra array creation might be unexpected.
Chris.

@the-thing
Copy link
Contributor Author

the-thing commented Aug 29, 2018

Actually I changed the default method to include all fields and the test that has failed made me realise that components have access to all message fields since component is just a logical structure and its implementation does not hold any fields.

This is definitely something not desired. I will close to pull request for the time being as I need to think what is a better solution to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants