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

Added Value (variant) Type #27487

Merged
merged 7 commits into from
Mar 12, 2022
Merged

Added Value (variant) Type #27487

merged 7 commits into from
Mar 12, 2022

Conversation

KrzysztofCwalina
Copy link
Member

@KrzysztofCwalina KrzysztofCwalina commented Mar 10, 2022

From time to time we run into cases where we want to store a heterogenous collection of values. This type allows to store such values while minimizing boxing. The type was developed by @JeremyKuhne. The original idea was that this type ends up in the BCL, but the idea was rejected.

Here is an example of a very similar type in a track2 library: https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/timeseriesinsights/Azure.IoT.TimeSeriesInsights/src/Models/TimeSeriesValue.cs

@ghost ghost added the Azure.Core label Mar 10, 2022
@check-enforcer
Copy link

This pull request is protected by Check Enforcer.
For more information about how to run a pipeline against this pull request, see this.

@azure-sdk
Copy link
Collaborator

API changes have been detected in Azure.Core.Experimental. You can review API changes here

@annelo-msft
Copy link
Member

From time to time we run into cases where we want to store a heterogenous collection of values.

Is there a sample showing how we would use it this way in an Azure SDK context?

@KrzysztofCwalina
Copy link
Member Author

@annelo-msft, this is how it's used today in one of the track 2 libraries: https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/timeseriesinsights/Azure.IoT.TimeSeriesInsights/src/Models/TimeSeriesValue.cs

Copy link
Member

@annelo-msft annelo-msft left a comment

Choose a reason for hiding this comment

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

Might make sense to add a few docstrings for key APIs on Value. Also, I defer to you on whether we have concerns about calls to Unsafe.

@KrzysztofCwalina KrzysztofCwalina marked this pull request as ready for review March 11, 2022 20:47
@JoshLove-msft JoshLove-msft enabled auto-merge (squash) March 12, 2022 01:04
@JoshLove-msft JoshLove-msft merged commit 65fc0ad into Azure:main Mar 12, 2022
@JeremyKuhne
Copy link

FYI: I remembered why I didn't store Type for null values. It does, of course, make it more performant. The key reason was I wanted to keep the semantics the same as casting to/from object. You could support storing the passed in Type information, but it would add non-trivial performance and cognitive overhead.

Possible options when you need to store a given type with no value:

  • Store the Type object itself
  • Use an enum that represents this `public enum NullOfType { SomeTypeName }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants