Moya: Proposal to split the TargetType protocol

[madnik]

As our TargetType protocol requirements are increasing. That results in large ServiceType classes or non protocol adapting extensions . I think it would be great enhancement if we split the TargetType protocol into multiple protocols using protocol inheritance. Still requiring to provide full specifications but allowing to split into multiple files with new protocol requirements. Although already it is possible to split specification fulfilment with non protocol adapting extensions of ServiceType I think this way is much neat.

I created example here at my fork. If the contributors agree with the proposal I can update demo and docs and create PR.

/// The protocol used to define the full specifications necessary for a `MoyaProvider`.
public protocol TargetType: TargetURLType, TargetHTTPMethodType, TargetParametersType, TargetSampleDataType, TargetTaskType, TargetValidationType {}

/// The protocol used to define the 'baseURL' and 'path' specifications.
public protocol TargetURLType {

  /// The target's base `URL`.
  var baseURL: URL { get }

  /// The path to be appended to `baseURL` to form the full `URL`.
  var path: String { get }
}

/// The protocol used to define the HTTP request method specifications.
public protocol TargetHTTPMethodType {

  /// The HTTP method used in the request.
  var method: Moya.Method { get }
}

/// The protocol used to define the parameter and encoding specifications.
public protocol TargetParametersType {

  /// The parameters to be incoded in the request.
  var parameters: [String: Any]? { get }

  /// The method used for parameter encoding.
  var parameterEncoding: ParameterEncoding { get }
}

/// The protocol used to define the sample data specifications.
public protocol TargetSampleDataType {

  /// Provides stub data for use in testing.
  var sampleData: Data { get }
}

/// The protocol used to define the HTTP task type specifications.
public protocol TargetTaskType {

  /// The type of HTTP task to be performed.
  var task: Task { get }
}

/// The protocol used to define the Alamofire validation requirements.
public protocol TargetValidationType {

  /// Whether or not to perform Alamofire validation. Defaults to `false`.
  var validate: Bool { get }
}


public extension TargetValidationType {
    var validate: Bool {
        return false
    }
}

Please review and feedback. Usage examples.

extension GitHubUserContent: TargetType {}

extension GitHubUserContent: TargetURLType {

  public var baseURL: URL { return URL(string: "https://raw.githubusercontent.com")! }
  public var path: String {
    switch self {
    case .downloadMoyaWebContent(let contentPath):
      return "/Moya/Moya/master/web/\(contentPath)"
    }
  }
}

extension GitHubUserContent: TargetHTTPMethodType, TargetParametersType {

  public var method: Moya.Method {
    switch self {
    case .downloadMoyaWebContent:
      return .get
    }
  }
  public var parameters: [String: Any]? {
    switch self {
    case .downloadMoyaWebContent:
      return nil
    }
  }
  public var parameterEncoding: ParameterEncoding {
    return URLEncoding.default
  }
}

extension GitHubUserContent: TargetSampleDataType {

  public var sampleData: Data {
    switch self {
    case .downloadMoyaWebContent:
      return animatedBirdData() as Data
    }
  }
}

extension GitHubUserContent: TargetValidationType {

  public var task: Task {
    switch self {
    case .downloadMoyaWebContent:
      return .download(.request(DefaultDownloadDestination))
    }
  }
}

[scottrhoyt]

This is an interesting proposal. I'm curious what other @Moya/contributors think.

My initial reaction is that this decomposition of TargetType would only be truly valuable if the separate protocols were useful for something other than being inherited by TargetType. If we are saying that all these things are necessary to have a TargetType and they aren't useful for anything else, then I maintain that they should be part of the TargetType itself.

The primary benefit of this decomposition is to provide a framework for structuring your implementation of TargetType, but it is a very loose framework at that since I could also do this:

extension GitHubUserContent: TargetType, TargetURLType, TargetHTTPMethodType, TargetParametersType, TargetSampleDataType, TargetValidationType {}

extension GitHubUserContent {

  public var baseURL: URL { return URL(string: "https://raw.githubusercontent.com")! }
  public var path: String {
    switch self {
    case .downloadMoyaWebContent(let contentPath):
      return "/Moya/Moya/master/web/\(contentPath)"
    }
  }
}

extension GitHubUserContent {

  public var method: Moya.Method {
    switch self {
    case .downloadMoyaWebContent:
      return .get
    }
  }
  public var parameters: [String: Any]? {
    switch self {
    case .downloadMoyaWebContent:
      return nil
    }
  }
  public var parameterEncoding: ParameterEncoding {
    return URLEncoding.default
  }
}

extension GitHubUserContent {

  public var sampleData: Data {
    switch self {
    case .downloadMoyaWebContent:
      return animatedBirdData() as Data
    }
  }
}

extension GitHubUserContent {

  public var task: Task {
    switch self {
    case .downloadMoyaWebContent:
      return .download(.request(DefaultDownloadDestination))
    }
  }
}

And neither of these strike me as having much more benefit than what is possible now:

extension GitHubUserContent: TargetType {}

// MARK: - URL construction

extension GitHubUserContent {

  public var baseURL: URL { return URL(string: "https://raw.githubusercontent.com")! }
  public var path: String {
    switch self {
    case .downloadMoyaWebContent(let contentPath):
      return "/Moya/Moya/master/web/\(contentPath)"
    }
  }
}

// MARK: - Method and Parameters

extension GitHubUserContent {

  public var method: Moya.Method {
    switch self {
    case .downloadMoyaWebContent:
      return .get
    }
  }
  public var parameters: [String: Any]? {
    switch self {
    case .downloadMoyaWebContent:
      return nil
    }
  }
  public var parameterEncoding: ParameterEncoding {
    return URLEncoding.default
  }
}

// MARK: - Sample Data

extension GitHubUserContent {

  public var sampleData: Data {
    switch self {
    case .downloadMoyaWebContent:
      return animatedBirdData() as Data
    }
  }
}

// MARK: - Tasks

extension GitHubUserContent {

  public var task: Task {
    switch self {
    case .downloadMoyaWebContent:
      return .download(.request(DefaultDownloadDestination))
    }
  }
}

[scottrhoyt]

If the goal is to decrease the verbosity in a TargetType file, my recommendations would be to consider:

• Break up the API into multiple TargetTypes, separated by file
• Potentially separate large TargetTypes into multiple files. parameters and sampleData might be profitable opportunities to introduce another file.
• Use an approach similar to suggested in #861 for defaulting the trivial properties for a specific TargetType.

[leoneparise]

This idea is really good! But there are some required properties like baseURL, path, parameters and method that must be included on all targets.

Parameters like sampleData, task and parameterEncoding, as @scottrhoyt pointed, we should provide defaults if the Target doesn't implement them. That will make Moya much more flexible and easy to learn.

[madnik]

@scottrhoyt 's answer make sense to me.

decomposition of TargetType would only be truly valuable if the separate protocols were useful for something other than being inherited by TargetType.

Yes. I agree with that. I will rethink about this.
@leoneparise I think proposed approach still make those properties required. But I agree that specifications becomes loose as @scottrhoyt pointed out. That something we need to address if we decompose.
Thanks for the arguments with facts contributors.

[manas-chaudhari]

I felt this would be useful when I was trying to use structs instead of an enum for TargetType. For sharing implementations of few methods like baseUrl between the structs, I was planning to create a hierarchy of structs using inheritance, where parent struct will conform the TargetURLType protocol.

However, this approach felt too rigid. Because, the way the TargetType gets broken directly impacts on what can and can't be shared. I needed a flexible approach.
I took a protocol oriented approach by creating intermediate empty protocols and adding extension functions to them.

protocol GithubUrl { }

extension GithubUrl {
    var baseURL: URL { return URL(string: "https://raw.githubusercontent.com")! }
}

struct EventsApi: TargetType, GithubUrl {
    var path: String { return "events" }
}

Ultimately, the decomposition didn't feel like the right approach for this usecase.

[scottrhoyt]

Great! Thanks for following up @manas-chaudhari. That's actually an approach I wind up using as well!