Reason For Post
There are so many different solutions & examples on how to build a proper networking layer, but every app has different constraints, and design decisions are made based on trade-offs, leaving me uncertain about the quality of code I've written. If there are any Anti-Patterns, redundancies, or flat out bad solutions within my code that I have overlooked or simply lacked the knowledge to address, please do critique. This is a project I'd like to add to my portfolio, so I'm posting it here to get eyes on it, with some advice/tips.
Thanks for your time in advanced!
Some characteristics of my networking layer that I think could raise eyebrows:
Method contains a GETALL case, to indicate a list of data that must be fetched. I have not seen this in any of the open source code I've read. Is this a code smell?
enum Method {
case GET
/// Indicates how JSON response should be handled differently to abastract a list of entities
case GETALL
case PUT
case DELETE
}
I've made it, so each Swift Entity conforms to JSONable protocol, meaning it can be initialized with json and converted to json.
protocol JSONable {
init?(json: [String: AnyObject])
func toJSON() -> Data?
}
JSONable in practice with one of my entities:
struct User {
var id: String
var name: String
var location: String
var rating: Double
var keywords: NSArray
var profileImageUrl: String
}
extension User: JSONable {
init?(json: [String : AnyObject]) {
guard let id = json[Constant.id] as? String, let name = json[Constant.name] as? String, let location = json[Constant.location] as? String, let rating = json[Constant.rating] as? Double, let keywords = json[Constant.keywords] as? NSArray, let profileImageUrl = json[Constant.profileImageUrl] as? String else {
return nil
}
self.init(id: id, name: name, location: location, rating: rating, keywords: keywords, profileImageUrl: profileImageUrl)
}
func toJSON() -> Data? {
let data: [String: Any] = [Constant.id: id, Constant.name: name, Constant.location: location, Constant.rating: rating, Constant.keywords: keywords, Constant.profileImageUrl: profileImageUrl]
let jsonData = try? JSONSerialization.data(withJSONObject: data, options: [])
return jsonData
}
}
This allows me to use generics to initialize all my entities in my client- FirebaseAPI, after I retrieve JSON response. I also haven't seen this technique in the code I've read.
In the code below, notice how GETALL is implemented to flatten the list of JSON objects. Should I have to do this at all? Is there a better way to handle any type of Json structure response?
AND Entities are initialized generically, and returned as an Observable ( Using RxSwift ).
Do you sense any code smells?
/// Responsible for Making actual API requests & Handling response
/// Returns an observable object that conforms to JSONable protocol.
/// Entities that confrom to JSONable just means they can be initialized with json & transformed from swift to JSON.
func rx_fireRequest<Entity: JSONable>(_ endpoint: FirebaseEndpoint, ofType _: Entity.Type ) -> Observable<[Entity]> {
return Observable.create { [weak self] observer in
self?.session.dataTask(with: endpoint.request, completionHandler: { (data, response, error) in
/// Parse response from request.
let parsedResponse = Parser(data: data, response: response, error: error)
.parse()
switch parsedResponse {
case .error(let error):
observer.onError(error)
return
case .success(let data):
var entities = [Entity]()
switch endpoint.method {
/// Flatten JSON strucuture to retrieve a list of entities.
/// Denoted by 'GETALL' method.
case .GETALL:
/// Key (underscored) is unique identifier for each entity
/// value is k/v pairs of entity attributes.
for (_, value) in data {
if let value = value as? [String: AnyObject], let entity = Entity(json: value) {
entities.append(entity)
}
}
/// Force downcast for generic type inference.
observer.onNext(entities as! [Entity])
//observer.onCompleted()
/// All other methods return JSON that can be used to initialize JSONable entities
default:
if let entity = Entity(json: data) {
observer.onNext([entity] as! [Entity])
//observer.onCompleted()
} else {
observer.onError(NetworkError.initializationFailure)
}
}
}
}).resume()
return Disposables.create()
}
}
}
I manage different endpoints like so:
enum FirebaseEndpoint {
case saveUser(data: [String: AnyObject])
case fetchUser(id: String)
case removeUser(id: String)
case saveItem(data: [String: AnyObject])
case fetchItem(id: String)
case fetchItems
case removeItem(id: String)
case saveMessage(data: [String: AnyObject])
case fetchMessages(chatroomId: String)
case removeMessage(id: String)
}
extension FirebaseEndpoint: Endpoint {
var base: String {
// Add this as a constant to APP Secrts struct & dont include secrets file when pushed to github.
return "https://AppName.firebaseio.com"
}
var path: String {
switch self {
case .saveUser(let data): return "/\(Constant.users)/\(data[Constant.id])"
case .fetchUser(let id): return "/\(Constant.users)/\(id)"
case .removeUser(let id): return "/\(Constant.users)/\(id)"
case .saveItem(let data): return "/\(Constant.items)/\(data[Constant.id])"
case .fetchItem(let id): return "/\(Constant.items)/\(id)"
case .fetchItems: return "/\(Constant.items)"
case .removeItem(let id): return "/\(Constant.items)/\(id)"
case .saveMessage(let data): return "/\(Constant.messages)/\(data[Constant.id])"
case .fetchMessages(let chatroomId): return "\(Constant.messages)/\(chatroomId)"
case .removeMessage(let id): return "/\(Constant.messages)/\(id)"
}
}
var method: Method {
switch self {
case .fetchUser, .fetchItem: return .GET
case .fetchItems, .fetchMessages: return .GETALL
case .saveUser, .saveItem, .saveMessage: return .PUT
case .removeUser, .removeItem, .removeMessage: return .DELETE
}
}
var body: [String : AnyObject]? {
switch self {
case .saveItem(let data), .saveUser(let data), .saveMessage(let data): return data
default: return nil
}
}
}
Last thing, I'd like someone with professional eyes to look at is, how I use MVVM. I make all network requests from view model, which comes out looking something like this:
struct SearchViewModel {
// Outputs
var collectionItems: Observable<[Item]>
var error: Observable<Error>
init(controlValue: Observable<Int>, api: FirebaseAPI, user: User) {
let serverItems = controlValue
.map { ItemCategory(rawValue: $0) }
.filter { $0 != nil }.map { $0! }
.flatMap { api.rx_fetchItems(for: user, category: $0)
.materialize()
}
.filter { !$0.isCompleted }
.shareReplayLatestWhileConnected()
collectionItems = serverItems.filter { $0.element != nil }.dematerialize()
error = serverItems.filter { $0.error != nil }.map { $0.error! }
}
}
In order to call api requests in a more expressive, formalized way, I am able to call api.rx_fetchItems(for:) inside flatmap above, because I extend FirebaseAPI to conform to FetchItemsAPI. I will probably have to follow the same pattern for most other requests.
extension FirebaseAPI: FetchItemsAPI {
// MARK: Fetch Items Protocol
func rx_fetchItems(for user: User, category: ItemCategory) -> Observable<[Item]> {
// fetched items returns all items in database as Observable<[Item]>
let fetchedItems = rx_fireRequest(.fetchItems, ofType: Item.self)
switch category {
case .Local:
let localItems = fetchedItems
.flatMapLatest { (itemList) -> Observable<[Item]> in
return self.rx_localItems(user: user, items: itemList)
}
return localItems
case .RecentlyAdded:
// Compare current date to creation date of item. If its within 24 hours, It makes the cut.
let recentlyAddedItems = fetchedItems
.flatMapLatest { (itemList) -> Observable<[Item]> in
return self.rx_recentlyAddedItems(items: itemList)
}
return recentlyAddedItems
case .Trending:
let trendingItems = fetchedItems
.flatMapLatest { (itemList) -> Observable<[Item]> in
return self.rx_trendingItems(items: itemList)
}
return trendingItems
default:
let stubItem = Item(id: "DEFAULT", createdById: "createdBy", creationDate: 1.3, expirationDate: 2.4, title: "title", price: 2, info: "info", imageUrl: "url", bidCount: 4, location: "LA")
return Observable.just([stubItem])
}
}
// MARK: Helper Methods
private func rx_localItems(user: User, items: [Item]) -> Observable<[Item]> {
return Observable<[Item]>.create { observer in
observer.onNext(items.filter { $0.location == user.location }) // LA Matches stubs in db
return Disposables.create()
}
}
func rx_recentlyAddedItems(items: [Item]) -> Observable<[Item]> {
return Observable<[Item]>.create { observer in
let recentItems = items
.filter {
let now = Date(timeIntervalSinceReferenceDate: 0)
let creationDate = Date(timeIntervalSince1970: $0.creationDate)
if let hoursAgo = now.offset(from: creationDate, units: [.hour], maxUnits: 1) {
return Int(hoursAgo)! < 24
} else {
return false
}
}
observer.onNext(recentItems)
return Disposables.create()
}
}
func rx_trendingItems(items: [Item]) -> Observable<[Item]> {
return Observable<[Item]>.create { observer in
observer.onNext(items.filter { $0.bidCount > 8 })
return Disposables.create()
}
}
}
I'm attempting to follow SOLID principles, and level up with RxSWift + MVVM, so I'm still unsure about the best OOP design for clean, maintainable code.