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

Update the ToString format of the BaseUnits/Dimensions #1486

Merged
merged 2 commits into from
Jan 2, 2025

Conversation

lipchev
Copy link
Collaborator

@lipchev lipchev commented Jan 1, 2025

  • BaseUnits: no longer using the AbbeviationsCache, the new format is L=Meter, M=Kilogram, T=Second
  • BaseDimensions: the exponent moved inside the dimension-brackets: [Length][Time^-1]
  • BaseDimensions: minor performance improvements

As mentioned in #1452, the main motivation here is the removal of the potential side effects of accessing/loading the unit abbreviations (e.g. during the QuantityInfo construction)

- BaseUnits: no longer using the AbbeviationsCache
- BaseDimensions: the exponent moved inside the dimension-brackets
- BaseDimensions: minor performance improvements
@lipchev
Copy link
Collaborator Author

lipchev commented Jan 1, 2025

@angularsen Happy new year, here's an easy one which we've talked about in #1452

I wasn't 100% sure back then (and I never actually meant for it to be optimal), but I did a quick benchmark and the string.Join variant is just as fast (on net8.0) as the the StringBuilder with AppendFormat (but using much less allocations). The only variant in which the StringBuilder was slightly faster (but still more allocations) was this one (AI generated, haven't actually verified the output):

public string ToStringWithAppend()
{
    if (Equals(Undefined))
    {
        return "Undefined";
    }
    var sb = new StringBuilder();
    if (Length is not null)
    {
        sb.Append("[Length]: ").Append(Length).Append(", ");
    }
    if (Mass is not null)
    {
        sb.Append("[Mass]: ").Append(Mass).Append(", ");
    }
    if (Time is not null)
    {
        sb.Append("[Time]: ").Append(Time).Append(", ");
    }
    if (Current is not null)
    {
        sb.Append("[Current]: ").Append(Current).Append(", ");
    }
    if (Temperature is not null)
    {
        sb.Append("[Temperature]: ").Append(Temperature).Append(", ");
    }
    if (Amount is not null)
    {
        sb.Append("[Amount]: ").Append(Amount).Append(", ");
    }
    if (LuminousIntensity is not null)
    {
        sb.Append("[LuminousIntensity]: ").Append(LuminousIntensity).Append(", ");
    }
    if (sb.Length > 2)
    {
        sb.Length -= 2; // Remove the trailing ", "
    }
    return sb.ToString();
}

@lipchev lipchev changed the title Update the ToString for th BaseUnits/Dimensions Update the ToString for the BaseUnits/Dimensions Jan 1, 2025
@lipchev lipchev changed the title Update the ToString for the BaseUnits/Dimensions Update the ToString format of the BaseUnits/Dimensions Jan 1, 2025
Copy link
Owner

@angularsen angularsen left a comment

Choose a reason for hiding this comment

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

Looks good to me, a couple of suggestions

UnitsNet.Tests/BaseUnitsTests.cs Outdated Show resolved Hide resolved
UnitsNet/BaseDimensions.cs Show resolved Hide resolved
UnitsNet.Tests/BaseDimensionsTests.cs Show resolved Hide resolved
@angularsen angularsen merged commit 9090623 into angularsen:release/v6 Jan 2, 2025
1 check passed
@angularsen
Copy link
Owner

@angularsen Happy new year, here's an easy one which we've talked about in #1452

I meant to answer and forgot, happy new year @lipchev 🎉

I'm very glad you are around to learn from, and discuss and share ideas with, I really appreciate it ❤️
Also your energy and brilliance is exactly what this project needs to move things forward, I do not have as much time to spend on it as I would like to 🙇

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.

2 participants