Your method reads an object from a JSON file which is embedded in the application bundle, and aborts with a runtime error if that fails for any reason. That is fine: Bad embedded data are a programming error and should be detected early.
The return type of the parse()
method should not be an optional because it never returns nil
.
The custom error type is used to pass errors from the helper methods to the main method. That has two disadvantages:
It hides (possible more informative) errors thrown from other methods, e.g. the JSON decoder method.
The messages printed with fatalError()
show only the error type and an integer, for example
Fatal error: The operation couldn’t be completed. (MyApp.JSONError error 0.)
The second issue can be improved (see for example How to provide a localized description with an Error type in Swift? on Stack OverFlow) but I would go another route and pass on existing errors instead, where possible:
In parseJson()
you can pass the error which the JSON decoder already throws, instead of shadowing it by a custom error:
private func parseJson<T: Decodable>(data: Data, type: T.Type) throws -> T {
try JSONDecoder().decode(type.self, from: data)
}
And in getJsonContents()
you can use the (throwing) Data(contentsOf: url)
method to read the data, that is also simpler than reading into a string and then converting the string to data.
It remains to handle an error in Bundle.main.url(forResource:withExtension:)
, that method does not throw an error but returns an optional.
Without the do-catching in the helper methods everything fits neatly into the main method:
func parse<T: Decodable>(type: T.Type) -> T {
guard let url = Bundle.main.url(forResource: self.fileName, withExtension: "json") else {
fatalError("file not found")
}
do {
let data = try Data(contentsOf: url)
return try JSONDecoder().decode(type.self, from: data)
} catch {
fatalError("\(error.localizedDescription)")
}
}
You many also want to print the full error
and not its localizedDescription
because that can provide more information (and the string is not presented to the user of your program but only for diagnostic purposes).
Finally, I wonder if fileName
should really be an instance variable. If it is only used in the parse()
method and nowhere else then you can make it a parameter of a static method:
struct JSONService {
static func parse<T: Decodable>(type: T.Type, from fileName: String) -> T {
guard let url = Bundle.main.url(forResource: fileName, withExtension: "json") else {
fatalError("file not found")
}
do {
let data = try Data(contentsOf: url)
return try JSONDecoder().decode(type.self, from: data)
} catch {
fatalError("\(error)")
}
}
}
which would be called as
let value = JSONService.parse(type: MyType.self, from: "filename")
Embedding it into a struct JSONService
is still useful as it provides a “name space“ for its static methods. If there are only static methods then you can embed them in an enum JSONService
instead, because no instance of that type needs to be created (thanks to @Shadowrun for reminding me about that).