Skip to content

Clock feedback#1

Merged
da2ce7 merged 2 commits intoda2ce7:protocol-testsfrom
josecelano:protocol-tests-clock-feedback
Sep 9, 2022
Merged

Clock feedback#1
da2ce7 merged 2 commits intoda2ce7:protocol-testsfrom
josecelano:protocol-tests-clock-feedback

Conversation

@josecelano
Copy link
Copy Markdown

hi @da2ce7, this is my feedback for this.

It's not finished yet.

  • Prefer From trait over Into.
  • Rename clock
  • Allow mocking the time in tests with a given time.

Why did I rename the Clock?

The Clock return an instance of itself, and it's the time. I see the Clock as the service and the Time as the value object. In this implementation, you do not use the service, only a value object that instantiates by itself.

With the new name, you see:

image

With the previous name, you read:

image

It's a little weird that the variable current_time is a UnixClock.

@josecelano josecelano changed the title Protocol tests clock feedback Clock feedback Sep 9, 2022
@da2ce7 da2ce7 changed the base branch from main to protocol-tests September 9, 2022 11:01
@da2ce7 da2ce7 merged commit 2279f51 into da2ce7:protocol-tests Sep 9, 2022
@josecelano
Copy link
Copy Markdown
Author

I forgot to mention that I prefer System over Local to refer to the operating system time because it's the name used in the standard library.

@josecelano josecelano deleted the protocol-tests-clock-feedback branch January 16, 2023 12:03
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.

2 participants