Handle edge cases

If the givennumbers array is empty then the first loop will do nothing, but

computedNumbers.append(numbers[count - 1] * numbers[0])

aborts with a runtime exception. The correct result in this case would be to setcomputedNumbers to an empty array as well.

Better loops

This

var i = 0while i < count - 1 {    // ...    i = i + 1}

is shorter and better done as

for i in 0 ..< count - 1 {    // ...}

The advantages are

• The scope ofi is limited to the loop.
• i is aconstant.
• The range ofi is clearly seen right at the start of the loop.

In this particular case we iterate of adjacent array elements, which is conveniently by byzip()ping the array with a shifted view of itself:

var computedNumbers = zip(numbers, numbers.dropFirst()).map(*)computedNumbers.append(numbers[count - 1] * numbers[0])

If we append the first element to the shifted view then the complete operation reduces to

let computedNumbers = zip(numbers, [numbers.dropFirst(), numbers.prefix(1)].joined()).map(*)

Note that this handles also the case of an empty array gracefully.

Use constants if possible

Thenumbers array is never mutated, therefore it should be declared as aconstant withlet:

let numbers = [3, 1, 4, 2]

Using a constant

• makes it clear to the reader that this value is never mutated,
• helps to avoid unintentional mutation,
• possibly allows better compiler optimization.

Make it a function

If we put the functionality in a separate function then

• it becomes reusable,
• test cases can be added more easily,
• we can give it a meaningful name,
• we can add documentation.

In our case the function would simply be

/// Compute an array where each element in the numbers array is multipled by/// the next following element. The last element is multiplied by the first element./// - Parameter numbers: An array of integers.func circularAdjacentProducts(of numbers: [Int]) -> [Int] {    return zip(numbers, [numbers.dropFirst(), numbers.prefix(1)].joined()).map(*)}

and would then be used as

let numbers = [3, 1, 4, 2]let computedNumbers = circularAdjacentProducts(of: numbers)print(computedNumbers)

Make it generic on the element type

The same operation could be done with an array of floating point values (Float orDouble), or with other integer types (e.g.Int16) as long as the multiplication does not overflow. Therefore we can generalize it to array of type[T] whereT conforms to a protocol which required a multiplication operation.

A possible choice is theNumeric protocol which is adopted by all floating point and integer types:

func circularAdjacentProducts<T: Numeric>(of numbers: [T]) -> [T] {    return zip(numbers, [numbers.dropFirst(), numbers.prefix(1)].joined()).map(*)}

Example:

let floats = [3.3, 1.1, 4, 2.2]let computedFloats = circularAdjacentProducts(of: floats)print(computedFloats) // [3.63, 4.4, 8.8, 7.26]

Make it generic on the collection type

The next possible generalization is to apply this operation not only to arrays, but to arbitrary collections:

extension Collection where Element: Numeric {    func circularAdjacentProducts() -> [Element] {        return zip(self, [dropFirst(), prefix(1)].joined()).map(*)    }}

Example with anArraySlice:

let numbers = Array(1...10).dropFirst(3)print(numbers) // [4, 5, 6, 7, 8, 9, 10]let computedFloats = numbers.circularAdjacentProducts()print(computedFloats) // [20, 30, 42, 56, 72, 90, 40]

If I remember correctly, at the time that exercise had been given, using loops had not been covered, so you did very well to discover the while loop and how to use it.

The point of that exercise was to show you how tedious it is to write essentially the same code for each different element:

let numbers = [3, 1, 4, 2]var results = [Int]()results.append(numbers[0] * numbers[1])results.append(numbers[1] * numbers[2])results.append(numbers[2] * numbers[3])results.append(numbers[3] * numbers[0])// and even maybeprint(results[0])print(results[1])print(results[2])print(results[3])// to print individually

...and that introduces the reason for using loops.

So your solution already went way beyond what was asked, so give yourself a pat on the back for that.

Martin's answer gives excellent advice on loops and those will also be covered in the next step of the course, but don't concern yourself too much about the more advanced stuff (like generics) for now. They are an important part of what makes Swift so expressive and so enjoyable to use, but they are an advanced topic and come later, once the basics have been addressed.

• 1
  \$\begingroup\$Welcome toCode Review! This nicely explains whatnot to do but the question is asking whatto do instead. Can you give advice on how to improve the code in the question?\$\endgroup\$

  Null

  – Null

  2019-11-21 16:36:10 +00:00

  CommentedNov 21, 2019 at 16:36
• \$\begingroup\$I think the part 'Better Loops' in Martin's answer is sufficient for that. My answer was really only because I recognise the exercise from an online course, where you can submit your answers and I assume Michale may want to submit his answer and is wanting to improve it, which Martin's answer addresses. I just wanted to assure him that loops are not an expected part of that answer because the course hasn't yet introduced them. This exercise expects the answer in long form, without loops, as I showed, but the loop form passes too. If it's not an appropriate answer for here, I'm happy to delete.\$\endgroup\$

  Najinsky

  – Najinsky

  2019-11-21 17:07:32 +00:00

  CommentedNov 21, 2019 at 17:07

• beginner
• array
• swift