HackerRank, Cracking the Coding Interview: Arrays - Left Rotation

Question

I just wanted to get some constructive feedback on my solution for this problem taken from the HackerRank Cracking the Coding Interview series.

CTCI Arrays Left Rotation Problem Statement

#include <map>
#include <set>
#include <list>
#include <cmath>
#include <ctime>
#include <deque>
#include <queue>
#include <stack>
#include <string>
#include <bitset>
#include <cstdio>
#include <limits>
#include <vector>
#include <climits>
#include <cstring>
#include <cstdlib>
#include <fstream>
#include <numeric>
#include <sstream>
#include <iostream>
#include <algorithm>
#include <unordered_map>

using namespace std;

vector<int> array_left_rotation(vector<int> a, int n, int k) {
    vector<int> b(a.size());
    for(int i = 0, len = a.size(); i < len; i++) {
        int swapValue = (i + len - k) % len;
        b[swapValue] = a[i];
    }

    return b;
}

int main(){
    int n;
    int k;
    cin >> n >> k;
    vector<int> a(n);
    for(int a_i = 0;a_i < n;a_i++){
        cin >> a[a_i];
    }
    vector<int> output = array_left_rotation(a, n, k);
    for(int i = 0; i < n;i++)
        cout << output[i] << " ";
    cout << endl;
    return 0;
}

Answer 1

Don't use:

using namespace std;

See: Why is “using namespace std” considered bad practice?

Only include the headers you need. Also organizing them by length of name does not seem to have any benefits. Personally I group them by function others sort them alphabetically. Pick something logical.

Pass large objects by const reference rather than value.

vector<int> array_left_rotation(vector<int> const& a, int n, int k) {
                                      ///   ^^^^^^

If you pass by value as in the code above. The compiler needs to add code to copy the object from the main function into array_left_rotation().

Prefer pre-increment:

for(int i = 0, len = a.size(); i < len; i++)
                                     // ^^^ prefer ++i

When using integers it makes no difference. But for other types it can potentially make a difference. So by using the pre-increment version you guarantee that you always use the most efficient version no matter what the type.

Not sure why we add len here:

int swapValue = (i + len - k) % len;

This is exactly the same as:

int swapValue = (i - k) % len;

Prefer to always use braces: '{' and '}' when you have a sub statements.

for(int i = 0; i < n;i++) 
    cout << output[i] << " ";

I would have used this:

for(int i = 0; i < n;i++) {
    cout << output[i] << " ";
}

Prefer \n over std::endl. THe only difference between the two is that std::endl will flush the stream. Since the code will automatically flush the stream at appropriate points (doing it manually usually results in efficiencies).

Answer 2

• Use only necessary #includes.

  #include <vector>
  #include <iostream>
  

  are the only two you are actually using.

• The n argument to array_left_rotation is never used.

• Use correct types. int may not be enough to represent the array size and its indices. Their type is std::vector<int>::size_type.

• The wording of the problem statement suggests that the array shall be rotated in-place.