Moya: Using Structs (working fork)

[Matthijn]

First of, Moya looks great. At least most of it does. But the one single enum bugs me a bit. You have to define a single API endpoint in multiple parts of the same file. When the API grows, the enum gets really big since it is hard to divide it into multiple files.

I've made some changes in my fork of Moya, and perhaps this idea can be discussed here and maybe (when the idea is liked) it can be integrated in Moya itself. But it would break existing code using enums in the way I've implemented it currently.

The benefits of using structs are:

• You can easily divide your API definition accros multiple files
• All information regarding one single API Endpoint is contained within one place

Also I've made some extra changes which makes using the structs even nicer.

• Automatic generating of the parameters in the request based on the structure of the struct using reflection (optional, you can still define your own parameters if you choose so)
• Easy support for urls with formatting like: /user/{id}

I've got a full README.md on the Github page with some example code. I'd love to hear all of your opinions on it.

The biggest change in regard to the original version of Moya is the addition of the Smoya.swift file. Which is short for StructuredMoya. In there some protocols and extensions have been defined.

Next to that most of Moya remains the same. But there are some changes regarding generics to hook up the right enum. This has been removed and it accepts any TargetType now.

Also the baseURL has been removed from the TargetType and you must add that in your own endpoint. (The default endpoint has been removed). Since defining the baseURL in every struct seemed odd to me.

Again, I'd love to get some feedback. And am curious if people like this approach.

[ashfurrow]

Hey, this is pretty cool! Thanks for the kind words, too.

I'll have more time this weekend to look closer at it. I agree there are limitations in centralizing things, especially as APIs grow. I've broken things into multiple enums in a project, and we've been talking about structs for a little bit. The use of the reflection API is really clever.

I'm not sure if we can "turn the ship" at this point (the structure of the enums is... pretty engrained into Moya), but I'd love to borrow some of the ideas from this. We should then consider linking to it from our docs, if you're interested in maintaining it as a separate project.

[Matthijn]

Yeah, I can understand that it would be quite a breaking change to move from the current enum structure to using structs. Nice to hear that you like the idea. Love to hear what you have to say when you've taken a closer look.

And yeah, feel free to borrow whatever you like. Maybe a separate project is an idea. But perhaps we can think of something else.

[ashfurrow]

Hey! Sorry I didn't get to this over the weekend :grimacing: Work is very busy right now and I'm preparing for a few conferences.

@Moya/contributors Would anyone be free to take a look, give feedback, and open issues to discuss ideas we should consider adopting?

[colinta]

Instead of removing DefaultEndpointMapping, you could makeit easier to port by having that method accept the baseURL, and then return a closure:

-public final class func DefaultEndpointMapping(target: Target) -> Endpoint<Target> {
-    let url = target.baseURL.URLByAppendingPathComponent(target.path).absoluteString
-    return Endpoint(URL: url, sampleResponseClosure: {.NetworkResponse(200, target.sampleData)}, method: target.method, parameters: target.parameters)
-}
+public final class func DefaultEndpointMapping(baseURL: NSURL) -> (target: Target) -> Endpoint<Target> {
+    return { target in
+        let url = baseURL.URLByAppendingPathComponent(target.path).absoluteString
+        return Endpoint(URL: url, sampleResponseClosure: {.NetworkResponse(200, target.sampleData)}, method: target.method, parameters: target.parameters)
+    }
+}

I like that your fork's changes are pretty small - but important question: does the current enum-based system still work?

I know you're critical of it, but... well, you're an army of ~2 at the moment. As long as we stay compatible with using enums to store the endpoints, I think this is great! What I mean is: it's great to support _both_, but if we're just going to support one or the other, I vote that we stick with the enum.

[sunshinejr]

It looks neat! I also thought about structures, but I really like enums' type safety. I'll take a closer look today/tomorrow if we could use something from it @ashfurrow. Also, great job @Matthijn 💣

[Matthijn]

@colinta That is a nice approach to the baseUrl. But since it is already part of the enum itself that would be kinda duplicate if you use the enum.

Also, In my current implementation the enum does not work anymore. Since with the enum you use generics to determine which enum cases can be passed. E.g: let service = new MoyaService<MyServiceEnum>(); and then service.request(.somethingInTheEnum()).

In my Struct based solution there is no generic MoyaService. You just create an instance like: let service = MoyaService(); and then you can pass _any_ TargetType into the .request() method.

---

You could make a "StructMoyaService" which is not generic and an "EnumMoyaService" which is. Where they both inherit from (a non generic) "MoyaService" which will contain the bulk of the code which can be shared (which is most). But I don't think that is the way you want to go.

But I can't think of any way from the top of my head to support both in an elegant way.

@sunshinejr Thanks!

[gregpardo]

I'm going to +1 this idea. I love Moya but while using it, one of my biggest frustrations was when working with/adding an endpoint... I would have to jump around these very large sections to get to the different parts of the enums. I think this is really great to have it all contained in a single place.

[wattson12]

Interesting discussion. I'd prefer enums still for a couple of reasons:

• I like how enums give my endpoints structure (e.g. AuthTarget.SignIn and ContentTarget.LatestPosts)
• While the enum code has more boilerplate, it has less magic and so less of an understanding curve which is important to me in an open source library

That said I really like a couple of points in the structs (non generic providers especially) and would be interested in seeing how we could get some of them into Moya

[Matthijn]

@watson You could of course use container structs to group certain targets together if you so choose.

[wattson12]

that's true, but it's more work for my use case which at the moment is a few different services with a few endpoints each (i.e. a micro services backend)

i might be biased though, I do love swift enums :)

[AvdLee]

I think I would love to see this kind of implementation too. For me personally, it makes it possible to fix my issue #383.

To make it more flexible, I mis an option to add other parameters to the struct which will not be automatically included in the query because of the reflection implementation.

[Matthijn]

@AvdLee If you don't add the ReflectiveParameters protocol extension to your Struct you can of course still define your own parameters variable within your struct, fine tuning on how you would like it to behave for that request.

---

Or maybe you meant that you would like to have some properties within the struct for other uses than generating the parameters then something like a parameterKeys could be introduced. That way you will have something like:

struct Foo: TargetType, ReflectiveParameters
{
    // The required properties for TargetType
    let path = "/foo"
    let method = Moya.Method.GET
    let sampleData = NSData();

    // Defines which properties must be added to the parameters through reflection
    let parameterKeys = ["baz", "bar"] // And for example when this is [ ] (default value) all properties will be added.

    // Will be added to the parameters
    var baz: String
    var bar: String

    // Will never be added
    var bak: Int
}

Instead of whitelisting blacklisting could be a possibility as wel.

[nilswx]

Can't wait for something like this to be added to Moya, or to see the project switch to something like this in a new major release. The monolith enum and the scattered stuff never made sense to me either, it just felt... 'bad' and that's a shame, because Moya is really nice and for the rest it actually tries to prevent ugly boilerplate code. I'd love to group relevant (self-containing) endpoint/target structs into .swift source files.

The use of Swift reflection is very interesting as well.

_subscribes to notifications for this issue_ . :shipit:

[ashfurrow]

I agree this is really interesting, I just want to provide a word of caution about radically changing Moya, and a work of assurance to our existing users who might be worried about large breaking changes.

I'm still unsure of the best approach to take here, but whatever we decide it will be done with careful consideration :+1:

[colinta]

Isn't it possible to just _wrap your struct_ in an enum value, and forward the TargetType calls to your struct?

struct UserResource {
  let method: Moya.Method = .GET
  // ... and so on ...
}

enum API {
  case User(UserResource)

  var path: String {
    switch self {
      case let User(resource):  return res.path.resolve()
    }
  }
}

You could probably take this a step further:

struct UserResource: Moya.TargetType {
 // ...
}

enum API {
  case Resource(Moya.TargetType)
}

This lets people who use enums (which is still _everyone_ at the moment) to continue without major refactors, but also lets you use your struct in the way you've proposed.

[ashfurrow]

@colinta that is _super_ cool. I think bringing that down deeper into Moya to have first-party support would be great, but at the very least that's a great solution _for now_ and we should probably document this idea :100: :100: :100:

The first-party support could look something like a middleware between the Moya Provider type and Target type. Something that generates an Endpoint (which really _doesn't_ need to be a generic at all). I'm just spit-balling, but having a middle layer that lets users use either sounds like an ideal world.

And I think it's worth noting that enums _are_ a great solution – that's why we used them in the first place. It just happens that they don't scale well. On the other hand, structs _do_ scale well, but spread domain knowledge of how an API works across several files (that's good for experts but can overwhelm beginners). Each approach has its advantages, so while an ideal world where either works is hard to imagine (and implement!) I think it may be worth striving for.

[orta]

:+1: - last year I mentioned that the current structure of Moya might not be able to handle our largest iOS project, with these ideas, it should be able to I think

[steam]

Its been interesting watching this discussion. As @colinta and I work on the same project that uses Moya I haven't commented because we share a similar love for enums and I share his perspective. The possibility of the project ditching them concerns me though so I figured I better speak up.

The enum pattern introduced to us by Moya very early on has helped guide a lot of our decisions in our large and mature swift app. We love using enums for just about everything we can. They express a large (and complete) set of information in a very concise package.

However, we ran into the same issue that @ashfurrow mentions, they don't scale very well when there are a lot of endpoints. We haven't had a great solve until now. I like @colinta's struct in an enum suggestion. Its something that works within the existing Moya paradigm and may help solve the scale issue. I see a refactor to ElloAPI in the near future. :metal:

[KevinVitale]

I adopted Moya for a new project back in November, and quickly found enums to not scale very well, also.

I came up with a solution similar to what @colinta and others have shared, with a struct wrapping a single (internal) TargetType adopter. I also extend the struct as a SignalTypeProducer, with magnificent results!

Here is an example taken from my playground file:

XCPlaygroundPage.currentPage.needsIndefiniteExecution = true

// http://jsonplaceholder.typicode.com
CurrentAPIHost = APIHost(serverContext: .PlaceholderTest)
JSONRequest("users")
    .startWithNext {
        print($0) // Individually prints each 'user' JSON object
}

// https://api.guildwars2.com/v2
CurrentAPIHost = APIHost(serverContext: .GuildWars2)
JSONRequest("colors", parameters: ["ids":"all"])
    .startWithNext {
        print($0)  // Individually prints each 'color' JSON object
}

---

JSONRequest looks like this:

public func JSONRequest<T>(endpointPath: EndpointPath, method: ReactiveMoya.Method = .GET, parameters: [String:AnyObject]? = nil) -> JSONTarget<T> {
    let endpoint = JSONEndpoint.Request(endpointPath: endpointPath, method: method, parameters: parameters)
    return JSONTarget(endpoint: endpoint)
}

---

EDIT: EndpointPath is this:

// MARK: -
// MARK: Endpoint Path
// MARK: -
public protocol EndpointPath {
    var endpointPath: String { get }
}

public extension RawRepresentable where RawValue == String {
    public var endpointPath: String {
        return self.rawValue
    }
}

public extension String {
    public var endpointPath: String {
        return self
    }
}
extension String: EndpointPath { }

[ashfurrow]

I think a face-to-face chat (over the internet) would be helpful here, to pin down:

1. What we want.
2. How to implement it.
3. Who will implement it.
4. Migration requirements.

I've scheduled a poll here for Thursday, March 10th: http://doodle.com/poll/ef4kbz3h8zi3crmz Sorry it can't be earlier, but I'm travelling until then. The times there are in EST, so go ahead and let me know when we can help this public hangout.

[ashfurrow]

OK, going to have a hangout tomorrow at 2pm EST. I'll post a Google Hangout link here.

[colinta]

I forgot to reply to the poll, but I'll be there! :-D

[ashfurrow]

OK, I've set up a hangout here: https://hangouts.google.com/hangouts/_/artsymail.com/moyahangout

Thought about recording it, but decided not to so people can speak freely. I'll summarize our discussion afterward and post here.

[ashfurrow]

Ok, here are the notes from the hangout:

We're going to continue using Moya's enum-first philosophy and @colinta's wrap-a-struct-in-an-enum solution. It has no breaking changes on the face value (more that shortly) and keeps with our existing philosophy (which I think would be unwise to change at this point).

@sunshinejr is going to send a PR with they're solution and we can discuss any technical problems and possible breaking changes there. @colinta has agreed to review the PR.

We also need documentation for what/why/how to do this, references _to_ that documentation added throughout our existing docs, and changes to Moya's documenting comments that reference the "Target enum." I can take care of the documentation, as well as adding a struct demo to our existing demo app.

How well or poorly an enum scales to handle an API is subjective; it varies from person to person, and even from endpoint to endpoint. It might make sense for a project to use both, for example. I like this technical approach because it maintains our current philosophy but provides flexibility for anyone who needs it.

@orta @colinta @sunshinejr Thanks again for taking the time to discuss this, and to @Matthijn and others who have brought up this idea. I must admit I was initially sceptical of this idea, but a working fork and this discussion have helped clarify the needs of Moya users. Thank you to everyone who participated :bow:

[colinta]

Thanks for continuing to encourage such a community-based project, @ashfurrow! Looking forward to see where this goes.

[KevinVitale]

I had a scheduling conflict, and so I appreciate the summary, @ashfurrow.

Having missed my opportunity to participate, I understand things are in motion; however, I'd still like to share my struct solution (KevinVitale/Shelley). It's promising to read so much of what motivated me reflected in the meeting notes, and as such, I'm looking forward to seeing what comes of it.

Thanks for such a great project, all.

[ashfurrow]

I think we've addressed this in #427, pending some documentation updates in #428 and #429. I'm going to close this issue, but if anyone has any further feedback, please feel free to comment where you think is appropriate; we can re-open this issue or open new ones, so don't be shy!