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

Add Beats::fromByteArray() static method #4233

Merged
merged 2 commits into from
Aug 19, 2021
Merged

Add Beats::fromByteArray() static method #4233

merged 2 commits into from
Aug 19, 2021

Conversation

Holzhaus
Copy link
Member

Make beats serialization/deserialization symmetric (there already is toByteArray()).

@Holzhaus
Beats: Rename deserialization functions to fromByteArray()
6ae4c0a 
The old naming was confusing because it made it hard to distinguish the
deserializion function from equally named creation functions.
@Holzhaus
Beats: Move deserialization logic from BeatFactory class
8299a06
@coveralls
Copy link

Pull Request Test Coverage Report for Build 1147780158

  • 7 of 17 (41.18%) changed or added relevant lines in 3 files are covered.
  • 12 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.009%) to 25.946%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/track/beatgrid.cpp 0 1 0.0%
src/track/beatmap.cpp 0 1 0.0%
src/track/beats.cpp 7 15 46.67%
Files with Coverage Reduction New Missed Lines %
src/engine/enginevumeter.cpp 1 90.24%
src/engine/cachingreader/cachingreader.cpp 11 71.01%
Totals Coverage Status
Change from base Build 1146568621: -0.009%
Covered Lines: 19992
Relevant Lines: 77052
💛 - Coveralls

uklotzde
uklotzde approved these changes Aug 19, 2021
View reviewed changes
Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

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

Good idea to increase the cohesion and simplify the code.

The BeatFactory utility class is of no use. The static methods could either be moved into Beats or defined as free functions. Step by step.

@uklotzde
Copy link
Contributor

LGTM

@uklotzde uklotzde merged commit ccef574 into mixxxdj:main Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@uklotzde uklotzde uklotzde approved these changes

Assignees
No one assigned
Labels
library
Projects
Beats Refactoring
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Holzhaus @coveralls @uklotzde