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

New generic DataType.BINARY #482

Merged
merged 4 commits into from
Jul 7, 2024
Merged

Conversation

oswaldobapvicjr
Copy link
Contributor

@oswaldobapvicjr oswaldobapvicjr commented Jun 21, 2024

This PR introduces a new DataType.BINARY to allow generic objects to be evaluated by special functions.

A new dedicated Converter -- BinaryConverter -- was created and added to the last position in the chain of converters in DefaultEvaluationValueConverter, allowing to convert any object that wasn't converted before, as per Discussion #481 (Working with XML Documents).

The existing behavior for objects that any converter could not convert was to throw an exception. I decided to preserve this behavior and let the new BinaryConverter be configurable in the ExpressionConfiguration so that the exception will not be thrown when the binary data type is enabled. I decided to let the initial state "false" for compatibility.

The BinaryConverter then reads the current configuration to determine whether or not to "convert" the object. To make that possible, I added the Expressionconfiguration as a new parameter to ConverterIfc.canConvert(...).

I finally added some JUnits for full coverage according to the repository rules.

Documentation files were also updated accordingly.

@uklimaschewski
Copy link
Collaborator

Thank you for the pull request and sorry for the late response!

I am a bit unhappy about changing the ConverterIfc interface, because this will break backward compatibility.
To do such things, a new major release version would be needed.

What do you think about just changing the DefaultEvaluationConverter instead?
Keep the list of converters, and after the loop, if no matching converter is found, either throw an exception or convert to binary if it is allowed:

//...
    for (ConverterIfc converter : converters) {
      if (converter.canConvert(object)) {
        return converter.convert(object, configuration);
      }
    }

    if (configuration.isBinaryAllowed()) {
        return EvaluationValue.binaryValue(object);
    }

    throw new IllegalArgumentException(
        "Unsupported data type '" + object.getClass().getName() + "'");

@oswaldobapvicjr
Copy link
Contributor Author

New commit available

@uklimaschewski uklimaschewski merged commit 4a43208 into ezylang:main Jul 7, 2024
1 check passed
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