current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 1 down vote favorite

I solve this problem in Swift. Looking for feedback on my code or a better solution:

enter image description here

import Foundation

// parses line to return array of numbers in string format
func readIntegers() -> [String] {
    return readLine()!.componentsSeparatedByString(" ").map { $0 }
}

// left rotates the array by a given input
func leftRotation() -> String {
    let input = readIntegers()
    let array = readIntegers()
    var newArray = [String](count: Int(input[0])!, repeatedValue: "")
    let (count, rotations) = (Int(input[0])!, Int(input[1])!)

    for index in 0..<count {
        // newIndex after rotation
        let newIndex = index + rotations
        if newIndex < count  { // up until count
            newArray[index] = array[newIndex]
        } else { // loops back to beginning index of array
            newArray[index] = array[newIndex - count]
        }
    }
    // joins the array
    return newArray.joinWithSeparator(" ")
}

print(leftRotation())
share|improve this question
up vote 1 down vote

First, whenever you see a phrase like "given x, perform y" you should think of the 'x' as inputs into a function that returns the result of 'y'.

So you should have something like func rotate(array array: [Int], left distance: Int) -> [Int] somewhere in your code. Getting the data from the user and displaying it should be separate from the function that does the work.

Second, the problem is underspecified. What if the distance to shift left is negative? Your solution will crash in the middle of the algorithm. It would be better to use the precondition function to make it explicit that this algorithm won't work with a negative count.

Lastly, Swift's array class has methods to make this much more elegant. You should be able to implement this without a loop & without any vars. (Read up on the functions dropFirst and dropLast.)

share|improve this answer
up vote 1 down vote

I am not a swift programmer so i will not be able to give you exact advice on how to improve the quality of your code but rather the algorithm you used to solve this challenge and nitpicks i have with your solution.

Firstly lets look at the question itself.

It asks you to print out the new array. why are you creating it and turning it back into a string?

That considered, you can simply perform 2 for loops

1 for loop up until the "break" where the array has overflown, and a second for loop for the rest of the untouched array.

like that

therfor we can simply print out the elements at location 

d to n

and then the elements from 

0 to d

This will shorten your code considerably and make it run a whole lot faster because no extra variables are needed and you arent doing array manipilation, its just a way to iterate over an array

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.