As Phrancis noted, there's no way for us to tell what an interviewer might want to learn from this. So this is just based on my impressions.
I don't know why this is wrapped in a main
function. The code appears written to run once, and take its input from the command line. But in that case, there's no reason to wrap it at all, really.
Conversely, if you do wrap it in a function, something has to actually call that function, but I don't see that anywhere.
So the code looks like it's supposed to be a standalone example that you just run - but it won't do anything unless you call main()
yourself. However, if the point is to make a function that can be used with other code, then:
- Don't call your function
main
, because that makes no sense
- Don't take over input and output. If I call a function to rotate an array, I'd expect to pass in the array to rotate, and get an array back. I don't expect to have to enter stuff manually, and just have the results printed back at me.
It'd make more sense if your code only handled the rotation itself, returned the rotated array, and left everything else to other code. Rotating the array is the core task here; not input handling or printing. That's just scaffolding to prove it works - you can still add all that, and make a self-contained little test script, but again, that's not the point of implementing array rotation.
So I'd focus on the rotation itself, put that in a function. Something like rotateArray(array, steps)
.
Now, for your actual implementation:
There's a straight-up bug: If the array is empty to begin with, shift()
will return undefined
which then that gets pushed back to the array n times. So for n rotations of an empty array, you'll get an array back with n values in it (which are all undefined
).
I wonder what'll happen if I try to rotate an array, say, 9,007,199,254,740,990 times. I'm betting it'd take a while (code's \$O(n)\$). And in the end, the rotated array might be back where it started, making all those trillions of shift
s and push
es pointless. So that seems like there are optimizations that can be made.
Using shift
and push
is a valid solution, just not a very efficient one. Rotating an array is the same as chopping it into two pieces and putting them together "backwards". That is what shift
and push
does too, they just can't do more than one element at a time.
Why can't I pass a negative rotation-count to rotate right instead?
Here's what I'd do:
function rotateArray(array, steps) {
// check if there's even something to rotate
if(array.length < 2) {
return array.slice(0); // always return a copy
}
// get the number of actual rotations to perform
var n = steps % array.length;
// check if there's any need to rotate
if(n === 0) {
return array.slice(0); // always return a copy
}
// slice and concat
if(n < 0) {
return array.slice(n).concat(array.slice(0, array.length+n));
} else {
return array.slice(n).concat(array.slice(0, n));
}
}
This code does a few simple checks to see if there's any reason to rotate the array at all, and, if there is, it does by concatenating two slices (code's \$O(1)\$), rather than going steps
number of iterations with shift
and push
.
And in case there's no reason to rotate anything, it just returns a copy of the input array (so that the output is always a new array, even if nothing needed to be rotated).