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

Add setters for property and values at jsonldobject, cloning the inst… #1695

Merged
merged 1 commit into from
Jan 4, 2023

Conversation

pope1838
Copy link
Contributor

@pope1838 pope1838 commented Jan 3, 2023

…ance and the metadata


def withProperty(property: String, value: JsonLDObject): JsonLDObject = _internal.withProperty(property, value)

def withStringPropertyCollection(property: String, values: Seq[String]): JsonLDObject =
Copy link
Contributor

Choose a reason for hiding this comment

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

can't we use method overloading instead of having a different name ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my original approach, but compilation failed :S


def withProperty(property: String, value: Int): JsonLDObject =
copyWithProperty(property.toIntField, buildInteger(value))

Copy link
Contributor

Choose a reason for hiding this comment

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

same as last comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't understood, sorry. In this case the method name is the same for all scalar, the type of the "value" param is the only difference, but that's to avoid do an internal match from "any" (and be forced to handle the unrecognized types)

@pope1838 pope1838 merged commit ab8a107 into develop Jan 4, 2023
@pope1838 pope1838 deleted the add-jsonldobject-property-setters branch January 4, 2023 00:19
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