I disagree. I find the first better in a number of respects (although there are 
certainly things you could do to optimise legibility).

Firstly, I can more clearly see all of the various file errors. I can easily 
see how many there are, which associated values they take, etc. That’s 
important when writing a switch statement, for instance.
Secondly, related data is group together. All of the localised descriptions are 
together, etc. In real code, things like localised descriptions will likely 
come from a common place - e.g. a localised string database.
Third: types. You never defined the type of any of these computed properties.

It’s a fun example, but personally I don’t find it convincing. Several of those 
values are closely related and could be grouped as a struct or tuple if brevity 
is so important. Do you really need separate “failureReason” and 
“recoverySuggestion” and “helpAnchor” properties? It seems to me like the first 
two are intended to always be displayed together, so you could reasonably group 
them. The helpAnchor seems to be some kind of reference - you could keep the 
separate accessor, but also use that same accessor to insert the value in to 
the aforementioned struct if you think it’s needed there, too (I would expect 
the compiler to be able to optimise the resulting switch statement away, given 
that it already knows the case of ’self’). You could also implement your 
LocalisedError accessor by forwarding to that same struct.

e.g:

struct ExtendedErrorInfo {
   var localisedDescription : String
   var failureReason : String
   var recoverySuggestion : String
   var anchor : String
}

enum FileError : Error {
   case notFound(URL)
   case accessDenied(URL)
   // ..etc

   var anchor : String {
      switch self {
         case .notFound(_):        return “404”
         case .accessDenied(_): return “DENIED"
      }
   }

  var extendedInfo : ExtendedErrorInfo {
      switch self {
         case .notFound(let url):
                return ExtendedErrorInfo(
                              localizedDescription: “Couldn’t find file at 
\(url)”
                              failureReason: "…”
                              recoverySuggestion: "…”
                              anchor: self.anchor)
         case .accessDenied(_):
                return ExtendedErrorInfo(
                              localizedDescription: “Access denied for file at 
\(url)”
                              failureReason: "…”
                              recoverySuggestion: "…”
                              anchor: self.anchor)
      }
  }
}

extension FileInfo : LocalizedError {
    var localizedDescription : String { 
        return extendedInfo.localizedDescription 
    }
}

That’s how I would do it if I cared about grouping that information together, 
in any case.

- Karl

> On 12 Oct 2016, at 21:21, Charles Srstka via swift-evolution 
> <swift-evolution@swift.org> wrote:
> 
>> On Oct 11, 2016, at 8:51 PM, Xiaodi Wu <xiaodi...@gmail.com 
>> <mailto:xiaodi...@gmail.com>> wrote:
>> 
>> On Tue, Oct 11, 2016 at 8:21 PM, Charles Srstka via swift-evolution 
>> <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
>>> On Oct 11, 2016, at 4:42 PM, Braeden Profile via swift-evolution 
>>> <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
>>> 
>>> enum RectSize
>>> {
>>>    let height:Int
>>>    let width:Int
>>>    case small(width: 30, height: 30)
>>>    case medium(width: 60, height: 60)
>>>    case large(width: 120, height: 120)
>>> }
>> 
>> I like the concept, but this doesn’t seem as flexible as it could be, and 
>> could get ugly when mixing these defaults with enum associated values. How 
>> about dynamic properties instead? Something like:
>> 
>> enum RectSize {
>>      var height: Int { get }
>>      var width: Int { get }
>> 
>>      case small {
>>              height { return 30 }
>>              width { return 30 }
>>      }
>> 
>>      case medium {
>>              height { return 60 }
>>              width { return 60 }
>>      }
>> 
>>      case large {
>>              height { return 120 }
>>              width { return 120 }
>>      }
>> 
>>      case custom(width: Int, height: Int) {
>>              height { return height }
>>              width { return width }
>>      }
>> }
>> 
>> I'd be interested in expanding raw values to accommodate other types, but 
>> computed properties are already possible:
>> 
>> ```
>> enum RectSize {
>>     case small, medium, large
>> 
>>     var height: Int {
>>         switch self {
>>         case .small:
>>             return 30
>>         case .medium:
>>             return 60
>>         case .large:
>>             return 120
>>         }
>>     }
>>     
>>     var width: Int {
>>         return height
>>     }
>> } 
>> ```
>> 
>> There have been off-and-on proposals to change the syntax from what it is 
>> currently, but none have been deemed a significant enough advantage to merit 
>> a change--even before source-breaking changes in Swift 3 were over. Keeping 
>> in mind that sugar is the lowest priority for Swift 4 (and not in scope for 
>> the current phase), what's the advantage of your proposed syntax for 
>> computed properties over the existing one?
> 
> It’s *possible*, but the syntax is incredibly verbose, unwieldy, and ugly. 
> Imagine an enum with lots of cases and lots of properties—and before you say 
> this is contrived, this already currently happens quite a lot with error 
> enums (pardon the badly written error messages; this is only for example):
> 
> enum FileError: Error, LocalizedError {
>     case notFound(url: URL)
>     case accessDenied(url: URL)
>     case incorrectFormat(url: URL)
>     case ioError(url: URL)
>     // ... imagine there are another 20 or so of these ...
>     
>     // Now, implement LocalizedError:
>     
>     var errorDescription: String? {
>         switch self {
>         case let .notFound(url: url):
>             return "Could not access the file \(url.lastPathComponent) 
> because it could not be found."
>         case let .accessDenied(url: url):
>             return "Could not access the file \(url.lastPathComponent) 
> because access was denied."
>         case let .incorrectFormat(url: url):
>             return "Could not access the file \(url.lastPathComponent) 
> because it was not in the expected format."
>         case let .ioError(url: url):
>             return "Could not access the file \(url.lastPathComponent) 
> because an I/O error occurred."
>         // ... etc ...
>         }
>     }
>     
>     var failureReason: String? {
>         switch self {
>         case let .notFound(url: url):
>             return "The file \(url.lastPathComponent) could not be found."
>         case let .accessDenied(url: url):
>             return "We do not have permission to view the file 
> \(url.lastPathComponent)"
>         case let .incorrectFormat(url: url):
>             return "The file \(url.lastPathComponent) was not in the expected 
> format."
>         case let .ioError(url: url):
>             return "An I/O error occurred while accessing the file 
> \(url.lastPathComponent)."
>         // ... etc ...
>         }
>     }
>     
>     var recoverySuggestion: String? {
>         switch self {
>         case .notFound:
>             return "Please locate the correct file and try again."
>         case .accessDenied:
>             return "You can change the file's permissions using the Finder's 
> Get Info window."
>         case .incorrectFormat:
>             return "The file may have become corrupt."
>         case .ioError:
>             return "Dear Lord, the hard drive may be failing."
>         // ... etc ...
>         }
>     }
>     
>     var helpAnchor: String? {
>         switch self {
>         case .notFound:
>             return "notFound"
>         case .accessDenied:
>             return "accessDenied"
>         case .incorrectFormat:
>             return "incorrectFormat"
>         case .ioError:
>             return "ioError"
>         // ... etc ...
>         }
>     }
>     
>     // Each of these errors references a file URL, so I may want a property 
> for that too
>     
>     var url: URL {
>         switch self {
>         case let .notFound(url: url):
>             return url
>         case let .accessDenied(url: url):
>             return url
>         case let .incorrectFormat(url: url):
>             return url
>         case let .ioError(url: url):
>             return url
>         // ... etc ...
>         }
>     }
> }
> 
> Look how ugly this is. Switch statements everywhere, related values separated 
> by really large amounts of space (and even more so if this error had more 
> cases, or if I’d implemented other error protocols like RecoverableError, 
> CustomNSError, etc.).
> 
> With the improved syntax, this could look something like this instead:
> 
> enum FileError: Error, LocalizedError {
>     var url: URL { get }
>     
>     case notFound(url: URL) {
>         errorDescription = "Could not access the file 
> \(url.lastPathComponent) because it could not be found."
>         failureReason = "The file \(url.lastPathComponent) could not be 
> found."
>         recoverySuggestion = "Please locate the correct file and try again."
>         helpAnchor = "notFound"
>         url = url
>     }
>     
>     case accessDenied(url: URL) {
>         errorDescription = "Could not access the file 
> \(url.lastPathComponent) because access was denied."
>         failureReason = "We do not have permission to view the file 
> \(url.lastPathComponent)"
>         recoverySuggestion = "You can change the file's permissions using the 
> Finder's Get Info window."
>         helpAnchor = "accessDenied"
>         url = url
>     }
>     
>     case incorrectFormat(url: URL) {
>         errorDescription = "Could not access the file 
> \(url.lastPathComponent) because it was not in the expected format."
>         failureReason = "The file \(url.lastPathComponent) was not in the 
> expected format."
>         recoverySuggestion = "The file may have become corrupt."
>         helpAnchor = "incorrectFormat"
>         url = url
>     }
>     
>     case ioError(url: URL) {
>         errorDescription = "Could not access the file 
> \(url.lastPathComponent) because an I/O error occurred."
>         failureReason = "An I/O error occurred while accessing the file 
> \(url.lastPathComponent)."
>         recoverySuggestion = "Dear Lord, the hard drive may be failing."
>         helpAnchor = "ioError"
>         url = url
>     }
>     
>     // ... etc ...
> }
> 
> I don’t think it can be denied that the second is orders of magnitude easier 
> to read and comprehend.
> 
> Charles
> 
> _______________________________________________
> swift-evolution mailing list
> swift-evolution@swift.org <mailto:swift-evolution@swift.org>
> https://lists.swift.org/mailman/listinfo/swift-evolution 
> <https://lists.swift.org/mailman/listinfo/swift-evolution>
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Reply via email to