Renaming NetworkRequest protocol?

[tylermilner]

I've been thinking about this for a while and wanted to open up discussion. I've personally never really been a fan of the Any...<T> convention used for type-erased structs. Instead of AnyNetworkRequest<T>, I think it would be nicer to be able to just write NetworkRequest<T>. To accomplish this, we would need to rename the NetworkRequest protocol to something else. Some ideas I have are:

• RequestRepresentable
• NetworkRequestRepresentable
• NetworkRequestProtocol

This would of course be a breaking change so it'd likely go into a 2.0.0 release. See the examples below for a peak of how this might look.

Defining something that conforms to RequestRepresentable:

struct CreatePostRequest: RequestRepresentable {
    // Define the model we want to get back
    typealias ResponseType = Post
    typealias ErrorType = AnyError

    // Define NetworkRequest property values
    var method: HTTP.Method = .post
    var url = URL(string: "https://jsonplaceholder.typicode.com/posts")!
    var queryParameters: [URLQueryItem]?
    var headers: [HTTP.HeaderKey: HTTP.HeaderValue]? = [.contentType: .applicationJSON]
    var body: Data? {
        let encoder = JSONEncoder()
        return try? encoder.encode(newPost)
    }

    // Define any custom properties needed
    private let newPost: NewPost

    // Initializer
    init(newPost: NewPost) {
        self.newPost = newPost
    }
}

Using NetworkRequest<T>:

let createPostRequest = NetworkRequest<Post>(method: .post,
                                             url: URL(string: "http://jsonplaceholder.typicode.com/posts")!,
                                             headers: [.contentType: .applicationJSON],
                                             body: postBody)

[wmcginty]

I like the idea a lot, my only reservation is moving away from the example of the standard library with AnyHashable.

I can be convinced either way.

[tylermilner]

Yeah, I've thought about that too. Apple has kind of made "Any" the standard here. Maybe even shortening to AnyRequest<T> instead of AnyNetworkRequest<T> (change NetworkRequest protocol to just Request as well) would be somewhat of an improvement.

[wmcginty]

I could get on board with that.

[wmcginty]

Is this something we want to try and include for 2.0?

[tylermilner]

I think so. I'd like more people to weigh-in with their thoughts though.

[earlgaspard]

I could get onboard with dropping the "Any".

[tylermilner]

So far I'm leaning towards:

• NetworkRequest protocol --> RequestRepresentable
• AnyNetworkRequest<T> struct --> Request<T>

Anyone else have any opinions? @BottleRocketStudios/ios-developers

[GrandLarseny]

To provide a bit of pushback, I believe the intended purpose is to differentiate naming a specific, concrete implementation of a protocol from the general idea of the protocol. I do agree that seeing Any as a prefix all over the place is ugly, so maybe there's some other way to convey the desired semantic meaning without resorting to a prefix.

It seems like above we're moving towards calling out the protocols instead of the concrete implementations. Could make a lot of sense.

[ngoleo]

I agree with renaming to RequestRepresentable to better communicate that it's a protocol.

For the "Any" request, I vote for OffTheShelfRequest 😁

[wmcginty]

I'm going to play devil's advocate here and suggest the following (taking the API naming guidelines into account strictly):

NetworkRequest ->Request AnyNetworkRequest<T> -> AnyRequest<T>

[WSTurner]

I like the idea of: NetworkRequest ->Request AnyNetworkRequest<T> ->AnyRequest<T>

Apple seems pretty consistent with their naming of type-erased values and I think retaining that consistency is a good idea.

[wmcginty]

@tylermilner Do you think we have consensus to move forward? It looks to me like:

NetworkRequest -> Request AnyNetworkRequest<T> -> AnyRequest<T>

[tylermilner]

@wmcginty Yeah, I think that's what we'll do. AFAIK, that should be one of the last changes that we need to release 2.0.0.

[wmcginty]

Under review in #52

[wmcginty]

Merged in #52