-
Notifications
You must be signed in to change notification settings - Fork 93
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
[NU-1836] Add casting and conversions docs #7124
base: release/1.18
Are you sure you want to change the base?
[NU-1836] Add casting and conversions docs #7124
Conversation
docs/scenarios_authoring/Spel.md
Outdated
- `isList`/`toList`/`toListOrNull` | ||
- `isMap`/`toMap`/`toMapOrNull` | ||
|
||
Functions with the prefix `is` check whether a type can be converted or cast to the appropriate type. Functions with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The behavior of functions prefixed with "is" is a surprise to me. Prefix "canBe" would fit better. Under normal circumstances I would not bother reading docs, because the name of the function offers a whole explanation.
Unfortunately, this is an incorrect explanation.
Now, I wonder whether this is an important difference. Whether something is "truly" a "Class", or just is convertable to the class. May be we do not need this distinction, I do not know. But I was really surprised.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to 'canBe'
docs/scenarios_authoring/Spel.md
Outdated
|
||
#### Explicit conversions | ||
|
||
Explicit conversions/casts are available in built-in functions and in utility classes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in built-in --> as built-in (??)
in utility classes - in which utility classes? #CONV?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
docs/scenarios_authoring/Spel.md
Outdated
the `to` prefix convert or cast a value to the desired type, and if the conversion fails, an exception is propagated | ||
further. Functions with the `toOrNull` prefix convert or cast a value to the desired type, and if conversion fails, a | ||
null value is returned. | ||
The `is`, `to` and `toOrNull` functions are slightly different in that they take a class name as a parameter. They first |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The is
, to
and toOrNull
functions are slightly different
What are they different from? More and more I see, that somewhere I missed an important point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
docs/scenarios_authoring/Spel.md
Outdated
conversion is applied. Below is the list of available conversion to classes: | ||
- BigDecimal | ||
- BigInteger | ||
- Boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the cast fails and there is a defined conversion ...
This is even more tricky for me, I simply do not understand. I am not sure whether the subtle differences you describe here come from the difference between casting (explicit) and conversion (often implicit, hidden); however you mention conversion functions, so I am lost.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rephrased it a little. I hope it's better or if it's not we will try to fix it.
5b6112c
to
4e4e3cb
Compare
@coderabbitai review please |
@arkadius: I will review the changes. ✅ Actions performedReview triggered.
|
WalkthroughThe modifications to Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
docs/scenarios_authoring/Spel.md (3)
267-267
: Add a comma for better readabilityAdd a comma after "Unknown data type" to improve sentence structure.
-Every unknown accessed field/element will produce `Unknown` data type which can be further navigated or [converted](#type-conversions) to a desired type. +Every unknown accessed field/element will produce `Unknown` data type, which can be further navigated or [converted](#type-conversions) to a desired type.🧰 Tools
🪛 LanguageTool
[uncategorized] ~267-~267: Possible missing comma found.
Context: ...eld/element will produceUnknown
data type which can be further navigated or [conv...(AI_HYDRA_LEO_MISSING_COMMA)
273-294
: Consider adding usage examplesThe explanation of conversion functions is clear, but adding practical examples would help users better understand when and how to use each type of function (canBe/to/toOrNull).
Consider adding examples like:
Examples: ```python canBeDouble("123") # returns true toDouble("123") # returns 123.0 toDoubleOrNull("abc") # returns null--- `345-345`: **Improve sentence structure** Revise the phrase for better readability. ```diff -Some conversions example: +Some examples of conversions:
🧰 Tools
🪛 LanguageTool
[uncategorized] ~345-~345: Possible missing preposition found.
Context: ...to the desired type). Some conversions example: | Input value ...(AI_HYDRA_LEO_MISSING_IN)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
docs/scenarios_authoring/Spel.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/scenarios_authoring/Spel.md
[uncategorized] ~267-~267: Possible missing comma found.
Context: ...eld/element will produce Unknown
data type which can be further navigated or [conv...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~345-~345: Possible missing preposition found.
Context: ...to the desired type). Some conversions example: | Input value ...
(AI_HYDRA_LEO_MISSING_IN)
🔇 Additional comments (2)
docs/scenarios_authoring/Spel.md (2)
271-272
: LGTM!
The introduction clearly sets up the distinction between implicit and explicit conversions.
295-336
: LGTM!
The conversion matrix is comprehensive and well-formatted, using emojis effectively to visualize conversion possibilities. The legend clearly explains the symbols used.
5672382
to
15fe64f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
docs/scenarios_authoring/Spel.md (3)
271-272
: Improve sentence structure for better readabilityConsider revising the sentence structure:
-It is possible to cast or convert from a type to another type and this can be done by implicit and explicit conversion. +It is possible to cast or convert from one type to another, and this can be done through implicit and explicit conversion.🧰 Tools
🪛 LanguageTool
[uncategorized] ~271-~271: Possible missing comma found.
Context: ... cast or convert from a type to another type and this can be done by implicit and ex...(AI_HYDRA_LEO_MISSING_COMMA)
273-296
: Consider adding a comparative exampleThe documentation clearly explains the different function variants. Consider adding a small example that demonstrates the behavior difference between
canBe
,to
, andtoOrNull
using the same input:Example: | Expression | Result | |------------------------|----------------------------- | | `"abc".canBeDouble` | false | | `"abc".toDouble` | throws exception | | `"abc".toDoubleOrNull` | null |
313-328
: Consider reorganizing the conversion matrixBased on the previous review comment from arkadius, consider swapping the rows and columns in the matrix. This would make it more intuitive to read as "Can I cast
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
docs/scenarios_authoring/Spel.md
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/scenarios_authoring/Spel.md
[uncategorized] ~271-~271: Possible missing comma found.
Context: ... cast or convert from a type to another type and this can be done by implicit and ex...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (3)
docs/scenarios_authoring/Spel.md (3)
267-267
: LGTM!
The addition provides valuable information about handling unknown field types.
297-309
: LGTM!
The examples effectively demonstrate various conversion scenarios, including success cases, null handling, and runtime exceptions.
Line range hint 334-371
: LGTM!
The documentation of utility classes and implicit conversions is clear, accurate, and well-supported with examples.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~271-~271: Possible missing comma found.
Context: ... cast or convert from a type to another type and this can be done by implicit and ex...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint
264-264: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
265-265: Expected: dash; Actual: asterisk
Unordered list style
(MD004, ul-style)
Describe your changes
Checklist before merge
Summary by CodeRabbit