snippetcppMinor
Count array inversions merge sort
Viewed 0 times
inversionsmergearraysortcount
Problem
How does it look? Comments, code correctness, etc.
//Function Prototypes
void buildArray(int anArray[]);
void printArray(int anArray[], int start, int end);
void sortArray(int start, int end, int masterArray[], int tempArray[]);
void mergeArray(int start, int split, int end, int masterArray[], int tempArray[]);
void bruteCount(int masterArray[]);
//Globals BAD, but gets the job done here
long long INVERSIONS_SORT = 0;
long long INVERSIONS_BRUTE = 0;
const int size = 100000;
void main() {
int masterArray[size];
int tempArray[size];
buildArray(masterArray);
bruteCount(masterArray);
cout > anArray[i];
}
//Prints the entire array
void printArray(int anArray[], int start, int end) {
for (int i = start; i split) {
for (int i = rightCount; i masterArray[j])
INVERSIONS_BRUTE++;
}Solution
-
The comment about the globals is noisy, but at least you're aware that they're bad. You can reduce the possibility of bugs by removing them, which is always good idea. Otherwise, it could suggest that this implementation needs reworking, especially if you do encounter some bugs.
Side-note:
-
I would recommend replacing the C-arrays with a storage container such as
You will also be able to invoke
-
For
The comment about the globals is noisy, but at least you're aware that they're bad. You can reduce the possibility of bugs by removing them, which is always good idea. Otherwise, it could suggest that this implementation needs reworking, especially if you do encounter some bugs.
Side-note:
size is technically a constant, not a global. It's okay to have constants in global scope as they cannot be modified elsewhere.-
I would recommend replacing the C-arrays with a storage container such as
std::vector or std::array (if you have C++11). In C++, it's not good to pass C-arrays to functions because they decay to pointers instead of passing the array itself. Storage containers, on the other hand, do not do this; they pass an object. That said, your implementation looks very C-like, although that's not relevant to the sorting. It's still worth knowing about storage containers in C++.You will also be able to invoke
operator= in one line to reassign the master array to the temp array instead of using a manual loop. These little things will help make your code cleaner.-
For
bruteCount():- Why are
iandjdeclared outside the loop statement? It's done differently elsewhere, so it should be done here as well.
- The loops and the
ifstatement should have curly braces, even if the body of a statement is just one line (also more a general recommendation). Not having them this way could hurt maintenance, in case you ever need to add or remove something.
Context
StackExchange Code Review Q#51585, answer score: 5
Revisions (0)
No revisions yet.