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.

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

QFJ-962: FieldMap.setField(int key, Field<?> field) does not properly convert values #226

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

WojciechZankowski
Copy link
Contributor

I have some concers:

  1. I wasn't really sure how to convert DateField / UtcTimeField / UtcDateField. Check how I did it.
  2. Abstract method convertToString requires changes from users that made custom extensions of Field class. I don't know it is an use case, but it was possible to do.

What I did:

  • I made Field abstract
  • Added abstract method convertToString
  • I added method setField(int, BytesField), but I am not sure about concept with BytesField. I have seen in comments and implementation that we don't convert BytesField into StringField, but for example there is method getField(int)
    StringField getField(int field) throws FieldNotFound {
        final StringField f = (StringField) fields.get(field);
        if (f == null) {
            throw new FieldNotFound(field);
        }
        return f;
    }

So it will fail anytime we would try to access bytes field that was set using one of methods

setField(int, BytesField)
setField(BytesField)
setBytes(int, byte[])

Anyway I wouldn't touch in scope of this defect.

Sorry for commit name and for small formatting changes. I used formatter and I didn't see them.

@chrjohn chrjohn added this to the QFJ 2.2.0 milestone Jul 29, 2019
@chrjohn chrjohn modified the milestones: QFJ 2.2.0, QFJ 3.0.0 Jan 9, 2020
@chrjohn chrjohn changed the title QFJ-962: FieldMap.setField(int key, Field<?> field) does not properly convert values QFJ-962: FieldMap.setField(int key, Field<?> field) does not properly convert values May 4, 2021
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