Skip to content

refactor!: transform StyleDefault into an object #2728

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

Merged
merged 2 commits into from
Jun 9, 2023

Conversation

tbouffard
Copy link
Member

@tbouffard tbouffard commented Jun 4, 2023

StyleDefault was previously an enum but there is no need to enumerate the values. In addition, the values store both number and string which is generally considered confusing and a bad practice. Switching to a regular object make things more explicit and fix several SonarCloud code smells related to the enum usage (mix value types and non literal values).

Notes

This PR applies something similar to MarkerIdentifiers and BpmnStyleIdentifiers with #1617 and #2716
See also https://www.typescriptlang.org/docs/handbook/enums.html#objects-vs-enums for more reasons to use objects instead of enum.
Using enum for StyleDefault, cause issues while trying to migrate to maxGraph because of the mixed number and string: it forced us to cast some values.

AFAIK, this impacts the way the HTML API is produced.
The API still lists the entries (there is no dedicated doc for entries) but in a different way. The entries are still available in the IDE.

before now
image image

Fixed SonarCloud code smells

TS rule typescript:S6583 Enum members should not mix value types

TypeScript provides both numeric and string-based enums. Members of enums can be assigned strings, numbers, both, or none, which default to numbers starting from zero. Although it is possible to mix the types of enum member values, it is generally considered confusing and a bad practice.
Enum members should be consistently assigned values of the same type, that is, strings or numbers.

typescript:S6550 Explicit enum value must only be a literal value (string, number, boolean, etc).

TypeScript allows all sorts of expressions to initialize enum members. However, as enums create their own scope, using identifiers can lead to unexpected results. The recommendation is thus to use only literal values when defining enum members.

Tasks

@tbouffard tbouffard added the refactoring Code refactoring label Jun 4, 2023
@github-actions
Copy link

github-actions bot commented Jun 4, 2023

♻️ PR Preview cdb1711 has been successfully destroyed since this PR has been closed.

🤖 By surge-preview

@github-actions
Copy link

github-actions bot commented Jun 4, 2023

♻️ PR Preview cdb1711 has been successfully destroyed since this PR has been closed.

🤖 By surge-preview

`StyleDefault` was previously an enum but there is no need to enumerate the values. In addition, the values store both
number and string which is generally considered confusing and a bad practice.
Switching to a regular object make things more explicit and fix 23 SonarCloud code smells.
@tbouffard tbouffard force-pushed the refactor/StyleDefault_as_object branch from 4b13c4a to 9d099f1 Compare June 7, 2023 13:21
@tbouffard tbouffard marked this pull request as ready for review June 8, 2023 14:21
@tbouffard tbouffard requested a review from csouchet June 8, 2023 14:21
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 9, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@tbouffard tbouffard merged commit e90167a into master Jun 9, 2023
@tbouffard tbouffard deleted the refactor/StyleDefault_as_object branch June 9, 2023 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Code refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants