Skip to content

octave insensitive chords#42

Merged
gfauredev merged 2 commits into
mainfrom
jisele
Jun 16, 2026
Merged

octave insensitive chords#42
gfauredev merged 2 commits into
mainfrom
jisele

Conversation

@gfauredev

Copy link
Copy Markdown
Member

No description provided.

@github-actions

Copy link
Copy Markdown

📊 Coverage Report

Copilot AI 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.

Pull request overview

This PR updates note validation to be octave-insensitive (aligning individual note validation with existing chord validation behavior) and adjusts logging output to avoid using std::println with file streams.

Changes:

  • Make AnswerValidator::valider ignore octave when comparing single notes.
  • Extend unit tests to cover matching notes across different octaves.
  • Write log lines to files using std::format + stream insertion instead of std::println(file, ...).

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/AnswerValidator.cpp Strips trailing octave digit and normalizes case before comparing notes.
test/AnswerValidatorTest.cpp Adds assertions for octave-insensitive single-note validation.
include/Logger.hpp Switches file logging to file << std::format(...) for compatibility.
test/ChallengeFactoryTest.cpp Minor formatting/indentation adjustment in the test.

Comment thread src/AnswerValidator.cpp
Comment on lines +22 to +23
std::transform(n1.begin(), n1.end(), n1.begin(), ::toupper);
std::transform(n2.begin(), n2.end(), n2.begin(), ::toupper);
Comment on lines 6 to +10
/// Compare notes jouées vs attendues (égalité stricte)
TEST_CASE("AnswerValidator::valider") {
AnswerValidator vn;
CHECK(vn.valider("c4", "c4"));
CHECK(vn.valider("c4", "c3")); // Différentes octaves
@gfauredev gfauredev merged commit d29c311 into main Jun 16, 2026
9 of 12 checks passed
@gfauredev gfauredev deleted the jisele branch June 16, 2026 22:54
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.

3 participants