Skip to main content
Code Review

Return to Answer

added 759 characters in body
Source Link
Martin R
  • 24.2k
  • 2
  • 38
  • 96

Your first method does not work correctly if the first row is empty:

print(max1(array: [[], [1, 2], [-3]])) // nil

This is difficult to salvage while avoiding an optional for the current maxval.

Your second method returns nil if any row is empty:

print(max2(array: [[1, 2], [], [3, 4]])) // nil

guard is a useful language feature to avoid the "optional pyramid of doom," or to "early exit" in exceptional cases. But in your max2() function it makes the program flow difficult to understand (and introduced an error, as we saw above). With plain old if-let-else it would look like this:

func max2(array: [[Int]]) -> Int? {
 var maxval: Int?
 for row in array {
 if let rowmax = row.max() {
 if let temp = maxval {
 if rowmax > temp { maxval = rowmax }
 } else {
 maxval = rowmax
 }
 }
 }
 return maxval
}

But actually the (explicit) handling of all the special cases of an empty array or empty rows can be avoided completely!

The Sequence.joined() can be applied to the nested array, it returns a (lazily evaluated) sequence of all concatenated rows.

This simplifies the implementation to

func max2D(array: [[Int]]) -> Int? {
 return array.joined().max()
}

Or more generic, for nested arrays of comparable elements:

func max2D<T>(array: [[T]]) -> T? where T: Comparable {
 return array.joined().max()
}

You may also consider if it is worth defining a function for that purpose, if you also can call

let myMax = my2DArray.joined.max()

directly.

Your first method does not work correctly if the first row is empty:

print(max1(array: [[], [1, 2], [-3]])) // nil

Your second method returns nil if any row is empty:

print(max2(array: [[1, 2], [], [3, 4]])) // nil

But actually the (explicit) handling of all the special cases of an empty array or empty rows can be avoided completely!

The Sequence.joined() can be applied to the nested array, it returns a (lazily evaluated) sequence of all concatenated rows.

This simplifies the implementation to

func max2D(array: [[Int]]) -> Int? {
 return array.joined().max()
}

Or more generic, for nested arrays of comparable elements:

func max2D<T>(array: [[T]]) -> T? where T: Comparable {
 return array.joined().max()
}

You may also consider if it is worth defining a function for that purpose, if you also can call

let myMax = my2DArray.joined.max()

directly.

Your first method does not work correctly if the first row is empty:

print(max1(array: [[], [1, 2], [-3]])) // nil

This is difficult to salvage while avoiding an optional for the current maxval.

Your second method returns nil if any row is empty:

print(max2(array: [[1, 2], [], [3, 4]])) // nil

guard is a useful language feature to avoid the "optional pyramid of doom," or to "early exit" in exceptional cases. But in your max2() function it makes the program flow difficult to understand (and introduced an error, as we saw above). With plain old if-let-else it would look like this:

func max2(array: [[Int]]) -> Int? {
 var maxval: Int?
 for row in array {
 if let rowmax = row.max() {
 if let temp = maxval {
 if rowmax > temp { maxval = rowmax }
 } else {
 maxval = rowmax
 }
 }
 }
 return maxval
}

But actually the (explicit) handling of all the special cases of an empty array or empty rows can be avoided completely!

The Sequence.joined() can be applied to the nested array, it returns a (lazily evaluated) sequence of all concatenated rows.

This simplifies the implementation to

func max2D(array: [[Int]]) -> Int? {
 return array.joined().max()
}

Or more generic, for nested arrays of comparable elements:

func max2D<T>(array: [[T]]) -> T? where T: Comparable {
 return array.joined().max()
}

You may also consider if it is worth defining a function for that purpose, if you also can call

let myMax = my2DArray.joined.max()

directly.

added 247 characters in body
Source Link
Martin R
  • 24.2k
  • 2
  • 38
  • 96

ThereYour first method does not work correctly if the first row is no need to explicitly iterate overempty:

print(max1(array: [[], [1, 2], [-3]])) // nil

Your second method returns nil if any row is empty:

print(max2(array: [[1, 2], [], [3, 4]])) // nil

But actually the rows and handle(explicit) handling of all the special cases of an empty array or empty rows. can be avoided completely!

The Sequence.joined() can be applied to the nested array, it returns a (lazily evaluated) sequence of all concatenated rows.

This simplifies the implementation to

func max2D(array: [[Int]]) -> Int? {
 return array.joined().max()
}

Or more generic, for nested arrays of comparable elements:

func max2D<T>(array: [[T]]) -> T? where T: Comparable {
 return array.joined().max()
}

You may also consider if it is worth defining a function for that purpose, if you also can call

let myMax = my2DArray.joined.max()

directly.

There is no need to explicitly iterate over the rows and handle all the cases of an empty array or empty rows.

The Sequence.joined() can be applied to the nested array, it returns a (lazily evaluated) sequence of all concatenated rows.

This simplifies the implementation to

func max2D(array: [[Int]]) -> Int? {
 return array.joined().max()
}

Or more generic, for nested arrays of comparable elements:

func max2D<T>(array: [[T]]) -> T? where T: Comparable {
 return array.joined().max()
}

You may also consider if it is worth defining a function for that purpose, if you also can call

let myMax = my2DArray.joined.max()

directly.

Your first method does not work correctly if the first row is empty:

print(max1(array: [[], [1, 2], [-3]])) // nil

Your second method returns nil if any row is empty:

print(max2(array: [[1, 2], [], [3, 4]])) // nil

But actually the (explicit) handling of all the special cases of an empty array or empty rows can be avoided completely!

The Sequence.joined() can be applied to the nested array, it returns a (lazily evaluated) sequence of all concatenated rows.

This simplifies the implementation to

func max2D(array: [[Int]]) -> Int? {
 return array.joined().max()
}

Or more generic, for nested arrays of comparable elements:

func max2D<T>(array: [[T]]) -> T? where T: Comparable {
 return array.joined().max()
}

You may also consider if it is worth defining a function for that purpose, if you also can call

let myMax = my2DArray.joined.max()

directly.

Source Link
Martin R
  • 24.2k
  • 2
  • 38
  • 96

There is no need to explicitly iterate over the rows and handle all the cases of an empty array or empty rows.

The Sequence.joined() can be applied to the nested array, it returns a (lazily evaluated) sequence of all concatenated rows.

This simplifies the implementation to

func max2D(array: [[Int]]) -> Int? {
 return array.joined().max()
}

Or more generic, for nested arrays of comparable elements:

func max2D<T>(array: [[T]]) -> T? where T: Comparable {
 return array.joined().max()
}

You may also consider if it is worth defining a function for that purpose, if you also can call

let myMax = my2DArray.joined.max()

directly.

default

AltStyle によって変換されたページ (->オリジナル) /