Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thoughts on using private structs in the file that declares the endpoints? #4

Closed
AndrewSB opened this issue Nov 8, 2016 · 4 comments

Comments

@AndrewSB
Copy link

AndrewSB commented Nov 8, 2016

This is awesome @devxoul 馃槃

I ran across Moya/Moya#447, and it made me wonder if it made any sense to use something like it in MoyaSugar, or if it makes more sense as is?

I'm really not sure, but the article included in the issue piqued my interest

@devxoul
Copy link
Owner

devxoul commented Nov 8, 2016

My poor English 馃槬 Could you please ask me again in plain English? I couldn't understand:

...wonder if it made any sense to use something like it in MoyaSugar, or if it makes more sense as is?

@AndrewSB
Copy link
Author

AndrewSB commented Nov 8, 2016

Don't blame yourself! Looking back, I was really unclear 馃檮

I was wondering if you saw any potential upside to changing parts of the implementation of MoyaSugar to use the pattern discussed in Moya/Moya#447, or if the potential upside is negligible

@devxoul
Copy link
Owner

devxoul commented Nov 8, 2016

@AndrewSB, oh I see. I think It would be something like:

extension GitHubAPI : SugarTargetType {

  var request: SugarRequest {
    switch self {
    case .userRepos(let owner):
      return SugarRequest(
        route: .get("/users/\(owner)/repos"),
        httpHeaderFields: [
          "Accept": "application/json"
        ]
      )

    case .createIssue(let owner, let repo, let title, let body):
      return SugarRequest(
        route: .post("/repos/\(owner)/\(repo)/issues"),
        parameters: [
          "title": title,
          "body": body,
        ],
        parameterEncoding: JSONEncoding(),
        httpHeaderFields: [
          "Accept": "application/json"
        ]
      )

    case .editIssue(let owner, let repo, let number, let title, let body):
      return SugarRequest(
        route: .patch("/repos/\(owner)/\(repo)/issues/\(number)"),
        parameters: [
          "title": title,
          "body": body,
        ],
        httpHeaderFields: [
          "Accept": "application/json"
        ]
      )
    }
  }

}

But I'm not sure this is better than before.

@AndrewSB
Copy link
Author

AndrewSB commented Nov 8, 2016

Yeah, you're right @devxoul - I like the current layout better than this one with SugarRequest structs, thank you for taking the time to give this some thought!

@AndrewSB AndrewSB closed this as completed Nov 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants