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

Logical error with American flag sorting #176

Open
Nakama3942 opened this issue Apr 5, 2022 · 0 comments
Open

Logical error with American flag sorting #176

Nakama3942 opened this issue Apr 5, 2022 · 0 comments

Comments

@Nakama3942
Copy link

For my project, I needed to implement sorting with the American flag. Searching the Internet for a code just to get acquainted with the algorithm (it's easier for me), I went to this repository. Focusing on it - I wrote the code in C ++. And it turned out that the algorithm can not count large numbers. I had an array of 16 elements and each element could be from 1 to 999. It turned out that it sorts only by units and tens. Replacing the line with the next - the code worked.

temp = (int)Math.log10(i) + 2; //86 line in the code

Then I decided to test an array with numbers up to 30,000. Here it has already sorted by units, tens and hundreds, not counting thousands. And only at this stage did I realize your next mistake. The algorithm requires the number of radix of the largest number, while for this same number you take not the number of the array, but the size of the array. As more and more numbers are given each time, the maximum will change to the last. In this code, everything depends not on the maximum number, but on the size of the array.
For the algorithm to work, you need to pass to the logarithm not the cell number of the array, but its value:

//The first variant
for (int i : unsorted)
{
	temp = (int)Math.log10(unsorted[i]) + 1;
	if(temp > max)
	{
		max = temp;
	}
}

In general, in my code I wrote as follows:

//The second variant
int getMaxNumberOfDigits()
{
	int count = 1;
	int value = ArrayProcessing<int>::getMax(Array, array_size); //My own function, but I think it is clear that it performs
	while(true)
	{
		value /= 10;
		if(value != 0)
		{
			count++;
		}
		else
		{
			break;
		}
	}
	return count;
}

My method is easier to understand. Your method sorts through each element of the array and calculates the number of its radix for each of them. And my method by the same search at once defines the largest element and only for one of it calculates quantity of radix. My method is faster because you don't have to count the number of radix for each element of the array.
I think Java would be the same mistake. I advise you to correct this mistake with either the first or the second variant. Here is a link to the file just in case.

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

No branches or pull requests

1 participant