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 constant value for StateName #1775

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

u1-liquid
Copy link

Use constant value for StateName.

With this, StateClass.StateName could be used in switch-case statement.

@u1-liquid u1-liquid marked this pull request as ready for review December 8, 2020 06:07
@Frazi1
Copy link

Frazi1 commented Dec 8, 2020

static readonly fields are evaluated at runtime when const — at compile time.

If state names are constants and if at some point a constant value is changed, it will require every Hangfire consumer to rebuild their code for these changes to take effect.

On the other hand, consumers will not have to do anything if it is a readonly field.

@u1-liquid
Copy link
Author

u1-liquid commented Dec 8, 2020

The state names are already stored into storage constantly, so if some point that value is changed, it means other consumers couldn't fetch jobs correctly until rebuild their codes for those changes to take effect.

I think there is no advantages about this point to keep StateName field to static readonly.

Also, this changes only affects to Hangfire.Core, So basically users who uses this library will NOT changes this part of codes,
If they need to change this kind of values, the state class should be defined in their code, which is not affected by this change.

static readonly fields are evaluated at runtime when const — at compile time.

If state names are constants and if at some point a constant value is changed, it will require every Hangfire consumer to rebuild their code for these changes to take effect.

On the other hand, consumers will not have to do anything if it is a readonly field.

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

Successfully merging this pull request may close these issues.

None yet

3 participants