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.

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

Fix loops issues #6086

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Treizzer
Copy link

@Treizzer Treizzer commented Nov 9, 2024

Modified while and for loop to fix the infinite loop and negative indices issues, respectively.
What happened before the fix:
Whether you have and array of numbers like -> Integer[] numbers = { 3, 12, 14, 23, 34, 45, 56, 67, 78, 89 };
When you send a value to "Key" greater than the last number in your array, e.g. 90 or greater, the program got into infinite loop and never return the expected "-1" for "key not found".
In case what the for-loop assignment a negative number to the variable "i" the condition will skip the negative index, and the for-loop will continue with the normal flow.

  • 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 code is formatted with clang-format -i --style=file path/to/your/file.java

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 73.51%. Comparing base (e63ba4b) to head (3f4ba78).

Files with missing lines Patch % Lines
...in/java/com/thealgorithms/searches/JumpSearch.java 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6086      +/-   ##
============================================
+ Coverage     73.50%   73.51%   +0.01%     
- Complexity     5097     5099       +2     
============================================
  Files           657      657              
  Lines         17625    17628       +3     
  Branches       3393     3394       +1     
============================================
+ Hits          12955    12960       +5     
  Misses         4166     4166              
+ Partials        504      502       -2     

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

@Treizzer Treizzer marked this pull request as ready for review November 9, 2024 03:11
@@ -41,12 +41,18 @@ public <T extends Comparable<T>> int find(T[] array, T key) {

int limit = blockSize;
// Jumping ahead to find the block where the key may be located
while (limit < length && key.compareTo(array[limit]) > 0) {
limit = Math.min(limit + blockSize, length - 1);
while (limit < length && key.compareTo(array[Math.min(limit, length - 1)]) > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The Math.min in while (limit < length && key.compareTo(array[Math.min(limit, length - 1)]) > 0) is a bit redundant because you already check limit < length in the while condition.

So it can be simplified to:

while (limit < length && key.compareTo(array[limit]) > 0) {
    limit += blockSize;
}

// Perform linear search within the identified block
for (int i = limit - blockSize; i <= limit && i < length; i++) {
if (i < 0) { // Skip negative indices
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like, the condition if (i < 0) continue; is unnecessary. Since limit starts at blockSize, limit - blockSize will always be >= 0.

}

// Handle the case where limit exceeds array length
limit = Math.min(limit, length);
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think to rewrite below loop to:

int start = Math.max(0, limit - blockSize);
int end = Math.min(limit, length);

// Perform linear search within the identified block
for (int i = start; i < end; i++) {
    if (array[i].equals(key)) {
        return i;
    }
}

@@ -41,12 +41,18 @@ public <T extends Comparable<T>> int find(T[] array, T key) {

int limit = blockSize;
// Jumping ahead to find the block where the key may be located
while (limit < length && key.compareTo(array[limit]) > 0) {
limit = Math.min(limit + blockSize, length - 1);
while (limit < length && key.compareTo(array[Math.min(limit, length - 1)]) > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add your test case, with infinite loop, to com.thealgorithms.searches.JumpSearchTest

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution!

@github-actions github-actions bot added the stale label Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants