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

TraitBase isImbued and isImbuedTo #818

Merged

Conversation

feltech
Copy link
Member

@feltech feltech commented Feb 21, 2023

Description

Closes #815

For symmetry with imbue and imbueTo methods, rename the isValid method to isImbued.

For C++ in particular, where performance is likely a concern, constructing a TraitBase-derived instance, just to check isImbued, is wasteful. So add a static isImbuedTo (for symmetry with imbueTo), which tests a TraitsData without unnecessary temporary
construction.

  • I have updated the release notes.
  • I have updated all relevant user documentation.

@feltech feltech self-assigned this Feb 21, 2023
@feltech feltech changed the title Work/815 trait base is valid improvements TraitBase isImbued and isImbuedTo Feb 21, 2023
Copy link
Contributor

@elliotcmorris elliotcmorris left a comment

Choose a reason for hiding this comment

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

Fabulouse'.

Copy link
Collaborator

@foundrytom foundrytom left a comment

Choose a reason for hiding this comment

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

Just a sneaky one that slipped the net:

ensuring the data holds that trait via `isValid`, or

@feltech feltech force-pushed the work/815-traitBaseIsValidImprovements branch from a3aeeae to 28d2303 Compare February 27, 2023 13:18
@feltech
Copy link
Member Author

feltech commented Feb 27, 2023

Just a sneaky one that slipped the net:

ensuring the data holds that trait via `isValid`, or

Fixed in 28d2303

@foundrytom foundrytom self-requested a review February 27, 2023 13:27
Copy link
Collaborator

@foundrytom foundrytom left a comment

Choose a reason for hiding this comment

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

Thanks @feltech, FFTSAMAYL

Part of OpenAssetIO#815. For symmetry with `imbue` and `imbueTo` methods, rename
the `isValid` method to `isImbued.`

Signed-off-by: David Feltell <david.feltell@foundry.com>
Closes OpenAssetIO#815. For C++ in particular, where performance is likely a
concern, constructing a `TraitBase`-derived instance, just to check
`isImbued`, is wasteful. So add a static `isImbuedTo` (for symmetry with
`imbueTo`), which tests a `TraitsData` without unnecessary temporary
construction.

Signed-off-by: David Feltell <david.feltell@foundry.com>
@feltech feltech force-pushed the work/815-traitBaseIsValidImprovements branch from 28d2303 to 9d5de0b Compare February 27, 2023 14:00
@foundrytom foundrytom merged commit bb070da into OpenAssetIO:main Feb 27, 2023
@feltech feltech deleted the work/815-traitBaseIsValidImprovements branch February 27, 2023 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Core] TraitBase isValid improvements
3 participants