HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcppMinor

Count array inversions merge sort

Submitted by: @import:stackexchange-codereview··
0
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: 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 i and j declared outside the loop statement? It's done differently elsewhere, so it should be done here as well.



  • The loops and the if statement 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.