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

DOPE-239: implemented all objectFunctions with extensions for cm and added tests #64

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

jansigi
Copy link
Collaborator

@jansigi jansigi commented Nov 12, 2024

No description provided.

@jansigi jansigi self-assigned this Nov 12, 2024
Copy link
Contributor

@martinagallati-ergon martinagallati-ergon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you purposefully leave out the OBJECT_FIELD function because of the ObjectEntry?

Base automatically changed from feature/dope-216-objects to main November 19, 2024 09:18
@jansigi jansigi force-pushed the feature/dope-239-object-funcitons branch from 2da79e1 to c5b7e69 Compare November 21, 2024 08:44
@jansigi jansigi force-pushed the feature/dope-239-object-funcitons branch from 4a74f87 to a3b24dc Compare January 24, 2025 17:02
@jansigi jansigi requested a review from pgruntz January 24, 2025 17:04
fun TypeExpression<ObjectType>.addAttribute(objectEntryPrimitive: ObjectEntryPrimitive<out ValidType>) =
ObjectAddExpression(this, objectEntryPrimitive)

fun TypeExpression<ObjectType>.addAttribute(key: TypeExpression<StringType>, value: TypeExpression<out ValidType>) =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are no build/integration tests, where you play around with different typeEpressions / nested-functions etc, right? Please add some

attributeKey: TypeExpression<StringType>,
) : FunctionExpression<ValidType>("OBJECT_FIELD", objectExpression, attributeKey)

fun TypeExpression<ObjectType>.objectField(key: TypeExpression<StringType>) =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the prefix object from the extension-funciton (You are not doing .objectConcat or similar as well).
Use a more descriptive naming, like getField()? (I'm don't know what the function should do)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(or something with Attribute?, below you use that term, would not use both for the same thing)

objectExpression: TypeExpression<ObjectType>,
) : FunctionExpression<ArrayType<ObjectType>>("OBJECT_INNER_PAIRS", objectExpression)

fun TypeExpression<ObjectType>.innerPairs() = ObjectInnerPairsExpression(this)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer to have function with a verb. Something like getInnerPairs()/createInnerPairs() or something (same for all of the functions)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should also add some integration-tests for objects

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.

3 participants