Skip to content

Fix: Resolved string bounding bug in UI (Fixes #1234)#7499

Open
Abhi13shek wants to merge 1 commit into
TheAlgorithms:masterfrom
Abhi13shek:main
Open

Fix: Resolved string bounding bug in UI (Fixes #1234)#7499
Abhi13shek wants to merge 1 commit into
TheAlgorithms:masterfrom
Abhi13shek:main

Conversation

@Abhi13shek

Copy link
Copy Markdown
  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized it.
  • All filenames are in PascalCase.
  • All functions and variable names follow Java naming conventions.
  • All new algorithms have a URL in their comments that points to Wikipedia or other similar explanations.
  • All new algorithms include a corresponding test class that validates their functionality.
  • All new code is formatted with clang-format -i --style=file path/to/your/file.java

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 63.63636% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.91%. Comparing base (ef986c4) to head (27f0f8d).

Files with missing lines Patch % Lines
...n/java/com/thealgorithms/backtracking/NQueens.java 20.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master    #7499   +/-   ##
=========================================
  Coverage     79.91%   79.91%           
  Complexity     7353     7353           
=========================================
  Files           811      811           
  Lines         23879    23879           
  Branches       4705     4705           
=========================================
+ Hits          19082    19084    +2     
+ Misses         4036     4035    -1     
+ Partials        761      760    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@prashantpiyush1111

Copy link
Copy Markdown
Contributor

Hi @Abhi13shek , thanks for the contribution! I reviewed the diff and have a few observations:

  1. Issue reference mismatch

The PR title says "Fixes #1234", but issue #1234 ("Can I add convex hull algorithms, some missing graph algos and hashing?") is a 2020 feature request that was auto-closed as stale in 2021. It has no connection to the changes in this PR — no convex hull, graph algorithm, or hashing code is touched here. Could you confirm the correct issue number this PR is meant to address?
2. Unrelated changes bundled together

This PR mixes two separate concerns:

NQueens.java: replacing System.out.println with Logger.info
StackPostfixNotation.java: refactoring Stack to ArrayDeque/Deque, plus try-with-resources for the Scanner

Per the contributing guidelines, it's usually cleaner to split unrelated changes into separate PRs — this makes review and rollback easier if something needs to be reverted later.
3. Test coverage gap

Codecov flags 4 missing lines in NQueens.java (63.6% patch coverage). It might be worth adding a test case that covers the logger branches (the "no solution found" and "arrangement found" paths) to close that gap.
4. Minor: empty log line

In NQueens.java, LOGGER.info("") (replacing the old blank System.out.println()) doesn't really add value — loggers print structured lines (timestamp/level), so an empty call just creates a noisy blank entry rather than visual spacing. Might be worth removing it.
cc @DenizAltunkapan @alxkm @yanglbme — flagging in case this needs a second look before review, since the issue reference doesn't seem to match the actual changes.

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