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.

Sign up for GitHub

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

Jump to bottom

Extract a new Frequency class from PositionedNote #107

Closed
albertms10 opened this issue May 12, 2023 · 0 comments · Fixed by #112
Closed

Extract a new Frequency class from PositionedNote #107

albertms10 opened this issue May 12, 2023 · 0 comments · Fixed by #112
Assignees
@plammens
Labels
refactor The code needs to be refactored
Milestone
Road to 0.7

Comments

@albertms10
Copy link
Owner

albertms10 commented May 12, 2023 •
edited
Loading

Description

  • Create a new Frequency class representing an absolute, physical pitch with corresponding methods (e.g., isHumanAudible, etc.).
  • Create a new .frequency getter from PositionedNote that returns a Frequency instance.

Before:

Note.c.inOctave(4).isHumanAudible

After:

Note.c.inOctave(4).equalTemperamentFrequency().isHumanAudible

Advantages

  • More semantically-precise API definition

Drawbacks

  • A bit more verbose syntax to access the same method.

Acknowledgements

@plammens for suggesting the change.
@albertms10 albertms10 added the refactor The code needs to be refactored label May 12, 2023
@albertms10 albertms10 added this to the Road to 0.7 milestone May 12, 2023
@plammens plammens self-assigned this May 12, 2023
plammens added a commit that referenced this issue May 12, 2023
@plammens
refactor: Extract Frequency
0997434 
This is to separate concerns: converting the positioned note to a frequency,
which could be done in different ways, e.g. equal tempered, pythagorean
(these methods belong to PositionedNote),
versus properties or functionality related to the frequency itself, such as
isHumanAudible or a hypothetical .play() method.

Closes #107
@plammens plammens mentioned this issue May 12, 2023
Merged
plammens added a commit that referenced this issue May 12, 2023
@plammens
refactor: Extract Frequency
4409f23 
This is to separate concerns: converting the positioned note to a frequency,
which could be done in different ways, e.g. equal tempered, pythagorean
(these methods belong to PositionedNote),
versus properties or functionality related to the frequency itself, such as
isHumanAudible or a hypothetical .play() method.

Closes #107
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@plammens plammens

Labels
refactor The code needs to be refactored
Projects
Music notes
Status: Done
Milestone
Road to 0.7
Development

Successfully merging a pull request may close this issue.

refactor: Extract Frequency albertms10/music_notes
2 participants
@plammens @albertms10