Skip to content

refactor: introduce enum class for MnType and clean up implementation accordingly - #5200

Merged
ogabrielides merged 3 commits into
dashpay:developfrom
knst:4k_hpmn-refac
Feb 16, 2023
Merged

refactor: introduce enum class for MnType and clean up implementation accordingly#5200
ogabrielides merged 3 commits into
dashpay:developfrom
knst:4k_hpmn-refac

Conversation

@knst

@knst knst commented Feb 14, 2023

Copy link
Copy Markdown
Collaborator

Issue being fixed or feature implemented

expressions like nType == MnType::HighPerformance.index look pretty confusing in current implementation of 4k HPMN.

Changing uint8_t index to enum class MnType : uint8_t give pros:

  • switch inside GetMnType() and any similar code will show a compiler warning if any type is missing.
  • instead "MnType::HighPerformance.index" you can write MnType::HighPerformance
  • you can remove confusing .index from MnType

But also Cons:

  • instead log("%d", nType) you need to write log("%d", static_cast<int>(nType));

What was done?

Introduced new enum class MnType and rewritten generating Regular/HighPerformance objects with params (description, collateral amount, etc).

Also were added attributes [[no_discard]] for related code.

How Has This Been Tested?

Run unit/functional tests

Breaking Changes

No breaking changes.

Checklist:

  • I have performed a self-review of my own code
  • I have assigned this pull request to a milestone

@knst knst added this to the 19 milestone Feb 14, 2023
@knst
knst requested a review from ogabrielides February 14, 2023 20:16
@knst
knst marked this pull request as ready for review February 14, 2023 20:17
ogabrielides
ogabrielides previously approved these changes Feb 15, 2023

@ogabrielides ogabrielides left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM.

@UdjinM6 UdjinM6 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, one nit

Comment thread src/evo/dmn_types.h Outdated
Comment thread src/evo/dmn_types.h Outdated

@UdjinM6 UdjinM6 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, utACK

@ogabrielides ogabrielides left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, utACK

@ogabrielides
ogabrielides merged commit 2083380 into dashpay:develop Feb 16, 2023
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.

4 participants