# HackerRank, Cracking the Coding Interview: Arrays - Left Rotation

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;
}


Don't use:

using namespace std;


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).

• without len added the result might be negative on lower i. If I'm not mistaken, modulus on negative values is implementation defined. – Incomputable Dec 19 '16 at 6:39
• Actually modulus is well defined. From: 5.6 Multiplicative operators [expr.mul] if the quotient a/b is representable in the type of the result, (a/b)*b + a%b is equal to a; otherwise, the behavior of both a/b and a%b is undefined. BUT you are correct for negative numbers it will generate negative values. – Martin York Dec 19 '16 at 7:02
• 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.