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

use GetValues method instead of GetNames #4828

Conversation

testfirstcoder
Copy link
Contributor

Some performance improvement.

public enum TypeScriptDateTimeType
{
    /// <summary>Uses the JavaScript Date object for date time handling.</summary>
    Date,
    /// <summary>Uses the Moment.js for date time handling.</summary>
    MomentJS,
    /// <summary>Uses the strings for date time handling (no conversion).</summary>
    String,
    /// <summary>Uses the Moment.js for date time with offset handling.</summary>
    OffsetMomentJS,
    /// <summary>Uses Luxon for date time handling.</summary>
    Luxon,
    /// <summary>Uses the DayJS.js for date time handling.</summary>
    DayJS,
}

[SimpleJob(RuntimeMoniker.Net80)]
[MemoryDiagnoser]
public class Benchmark
{
    [Benchmark]
    public void GetNames()
    {
        TypeScriptDateTimeType[] dateTimeTypes = Enum
            .GetNames(typeof(TypeScriptDateTimeType))
            .Select(t => (TypeScriptDateTimeType)Enum.Parse(typeof(TypeScriptDateTimeType), t))
            .ToArray();
    }

    [BenchmarkAttribute]
    public void GetValues_Explicit_Cast()
    {
        TypeScriptDateTimeType[] dateTimeTypes = (TypeScriptDateTimeType[])Enum.GetValues(typeof(TypeScriptDateTimeType));
    }

    [BenchmarkAttribute]
    public void GetValues_Cast_ToArray()
    {
        TypeScriptDateTimeType[] dateTimeTypes = Enum.GetValues(typeof(TypeScriptDateTimeType)).Cast<TypeScriptDateTimeType>().ToArray();
    }
}

void Main() => BenchmarkRunner.Run<Benchmark>();

// * Summary *

BenchmarkDotNet v0.13.12, Windows 10 (10.0.19045.4170/22H2/2022Update)
Intel Core i7-10700 CPU 2.90GHz, 1 CPU, 16 logical and 8 physical cores
.NET SDK 8.0.202
[Host] : .NET 8.0.3 (8.0.324.11423), X64 RyuJIT AVX2

Job=.NET 8.0 Runtime=.NET 8.0

Method Mean Error StdDev Gen0 Allocated
GetNames 517.95 ns 9.369 ns 7.824 ns 0.0372 312 B
GetValues_Explicit_Cast 96.60 ns 1.789 ns 1.674 ns 0.0057 48 B
GetValues_Cast_ToArray 137.05 ns 2.430 ns 2.154 ns 0.0114 96 B

@testfirstcoder testfirstcoder changed the title use GetValues method instead of GetNames use getvalues method instead of getnames Mar 21, 2024
@testfirstcoder testfirstcoder changed the title use getvalues method instead of getnames use GetValues method instead of GetNames Mar 21, 2024
@RicoSuter
Copy link
Owner

Is this really the same?
This is in the UI, does it even matter?

@testfirstcoder
Copy link
Contributor Author

Is this really the same?

{
    var dateTimeTypesInitial = Enum
            .GetNames(typeof(TypeScriptDateTimeType))
            .Select(t => (TypeScriptDateTimeType)Enum.Parse(typeof(TypeScriptDateTimeType), t))
            .ToArray();

    var dateTimeTypesRefactored = (TypeScriptDateTimeType[])Enum.GetValues(typeof(TypeScriptDateTimeType));

    Debug.Assert(dateTimeTypesInitial.GetType().Equals(dateTimeTypesRefactored.GetType()));

    Debug.Assert(dateTimeTypesInitial.SequenceEqual(dateTimeTypesRefactored));
}

does it even matter?

Not sure if but it's concise and more readable.

@RicoSuter RicoSuter merged commit 9641df8 into RicoSuter:master Mar 26, 2024
3 checks passed
Comment on lines +76 to +77
public TypeScriptDateTimeType[] DateTimeTypes { get; } =
(TypeScriptDateTimeType[])Enum.GetValues(typeof(TypeScriptDateTimeType));
Copy link
Contributor

Choose a reason for hiding this comment

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

One line

Copy link
Contributor

@0liver 0liver Jun 14, 2024

Choose a reason for hiding this comment

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

How about using the generic version Enum.GetValues() ?

Suggested change
public TypeScriptDateTimeType[] DateTimeTypes { get; } =
(TypeScriptDateTimeType[])Enum.GetValues(typeof(TypeScriptDateTimeType));
public TypeScriptDateTimeType[] DateTimeTypes { get; } = Enum.GetValues<TypeScriptDateTimeType>();

Copy link
Collaborator

Choose a reason for hiding this comment

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

It makes very little difference to comment a merged PR, the work has been finished.

Copy link
Contributor Author

@testfirstcoder testfirstcoder Jun 17, 2024

Choose a reason for hiding this comment

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

How about using the generic version Enum.GetValues() ?

According to the documentation the overloaded version of the generic method GetValues is available from the version .NET Core 5.0.

The current framework version of the project NSwagStudio is .NET 4.6.2.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great catch, @testfirstcoder! 😅

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.

5 participants