Giter Site home page Giter Site logo

Comments (23)

rayfix avatar rayfix commented on May 18, 2024

Guidelines are just that, and I like the changes and exceptions you have made. One that I am not sure about is

Service.resource(url:) → resourceWithURL(_:)

There is another guideline that says not to repeat type information. Which would make it:

Service.resource(url:) → resourceWith(_: NSURL)

I think the deprecation plan sounds good.

from siesta.

pcantrell avatar pcantrell commented on May 18, 2024

Thanks for the review, Ray.

You raise a good question about resourceWithURL(_:). My thinking is as follows:

There’s value in having a flavor of the method that accepts a string instead of an NSURL, because it can fold “nil string” and “invalid URL” into a single method call. The logic for that is mildly tangled if there’s only an NSURL flavor of the method, and is likely to be repeated a lot. So you want one version of the method that takes a string that's a URL.

However, there also needs to be a method that takes a string that’s a baseURL-relative path. At that point, there are two different Service.resource(String) methods. The “Compensate For Weak Type Information” section of the guidelines recommends adding clarifying words to method & param names in this situation:

resourceWithPath(String)
resourceWithURL(String?)
resourceWithURL(NSURL?)

That’s not bad, but I feel like resourceWithPath(_:) is kind of the default and should be just resource(_:). It’s super common, and I like how it reads in code:

func currentUser() -> Resource { return resource("/user") }

service.resource("/widgets").child(widgetID).addObserver(…)

But maybe I’m wrong about that, and the added clarity of resourceWithPath(_:) is preferable?

func currentUser() -> Resource { return resourceWithPath("/user") }

service.resourceWithPath("/widgets").child(widgetID).addObserver(…)

from siesta.

rayfix avatar rayfix commented on May 18, 2024

I had forgotten about the “Compensate For Weak Type Information” section. Thanks for reminding me. I like the way resource("/user") looks too. Getting pedantic here but if you call it resourceWithPath I would naively think I need to specify the full path: e.g. service.resourceWithPath("/widgets/\(widgetID)") which is wrong.

One thing I was wondering about was resources always seem to with a forward slash. Is the shorter resource("user") frowned upon?

from siesta.

pcantrell avatar pcantrell commented on May 18, 2024

One thing I was wondering about was resources always seem to with a forward slash. Is the shorter resource("user") frowned upon?

Nah, I left it open to both styles on purpose. It doesn't matter, since that method always creates something underneath the base URL (which is meant to prevent security holes caused by, say, an accidental hostname switch). I tend to prefer resource("/user") just because API docs usually include the leading slash.

from siesta.

pcantrell avatar pcantrell commented on May 18, 2024

Looking back over this list, I’m wondering about adding a prefix to the request hooks, maybe this:

resource.load()
    .onSuccess { data in print("Wow! Data!") }
    .onFailure { error in print("Oh, bummer.") }

…instead of this:

resource.load()
    .success { data in print("Wow! Data!") }
    .failure { error in print("Oh, bummer.") }

@rayfix, any others … gut reactions to that?

from siesta.

rayfix avatar rayfix commented on May 18, 2024

+1 Generally I am a fan less typing, but this seems like a good change. The on prefix makes it 100% clear that a closure callback follows (rather than say returning a bool or error type). I also like that it groups both related operations together alphabetically and how that works with autocomplete.

from siesta.

pcantrell avatar pcantrell commented on May 18, 2024

Good point about the autocomplete. Hadn’t even crossed my mind.

I’ve updated the gist to reflect “on” prefixes as well.

from siesta.

radex avatar radex commented on May 18, 2024

Without the ifNone: label, a call sounds as if the argument indicates the type — which is true, sort of, but confusing:
To my eyes, the guideline-breaking alternative reads best:

That's why it's a guideline, not a law!

I do agree that this:

resource.contentAsType(ifNone: placeholderImage)

Seems best. The rationale being, ifNone doesn't really describe the method (fundamentally the job is "cast content so that it appears as T"), it describes the parameter. Most of the time, the job of the method makes the first parameter obvious (hence the guideline), but here, it doesn't. So the parameter makes sense.

(Having said that, I think it would be worth rethinking this particular API either way, because it's really not obvious that the ifNone not only supplies the default value but also determines the type of the cast return value.)

Service.resource(url:) → resourceWithURL(_:)

Again, I think url: is better than WithURL in the name. The whole "fooWithType" convention is bleh to me. The only potential upside is that you have a different symbol for the method so you can reference it unambiguously. But I'd consider it a language limitation and I'm not sure if it matters here anyway.

Correct me if I'm wrong, but AFAICT the difference between resourceWithURL and resource is that the former takes an absolute URL, and the latter is relative to the service's base URL — right?

I feel like this semantic difference isn't well conveyed in the naming, and the fact that "URL" doesn't just mean "NSURL", as there's an override that takes an equivalent URL string, confuses matters even more.

What about resource(absoluteURL:) perhaps? Or maybe even resource(absolute:), since at call site the type is clear. The former is a bit more verbose than just (url:), but clearer about the intention, I think.

For this one, there’s not an obvious imperative verb rephrasing that isn’t either too verbose or less clear:

Configuration.beforeStartingRequest(_:) → same
The guideline-conforming executeBeforeStartingEachRequest or addPreRequestHook don’t read as well.

Agreed. I think the general rule is sound, but I think there's great value in this idiom as well, and the reason it works well is because it's followed by a closure. So it's going to look like this:

Configuration.beforeStartingRequest {
   ...
}

Which is super clear and nice. And in situations where you'd pass a reference to a method (not sure if applicable here), it's going to be clear too, as you'd generally make that an imperative verb phrase:

Configuration.beforeStartingRequest(doSomething)

Request.completion(:) → onCompletion(:)
Request.success(:) → onSuccess(:)
Request.newData(:) → onNewData(:)

+1 on that as well.

I'm wondering if a similar pattern wouldn't also work well here:

ResourceObserver.onResourceChange(...)
onResourceRequestProgress

This one also runs afoul of “Avoid abbreviations”:

Resource.withParam(_:) → same

I think this is fine. param is a common abbreviation, especially in HTTP/REST handling domain.

Service.baseURL → base

This is one of those cases where repeating type information might be preferable after all. And the reason why is that "URL" isn't just type information here, it also actually describes the thing the property is. And "base" works to describe "URL", not as a standalone noun.

At first glance "Service.base" seems to suggest that there's a class called Base, or perhaps ServiceBase, or that there's some fundamental concept of a "base". And there isn't. There's just a base URL.

from siesta.

pcantrell avatar pcantrell commented on May 18, 2024

Thanks for this in-depth review, Radek!

The rationale being, ifNone doesn't really describe the method … it describes the parameter. Most of the time, the job of the method makes the first parameter obvious (hence the guideline), but here, it doesn't. So the parameter makes sense.

I like this as a counterpoint to the Apple guidelines. I’m still tentative about how I feel about that particular rule.

Having said that, I think it would be worth rethinking this particular API either way, because it's really not obvious that the ifNone not only supplies the default value but also determines the type of the cast return value.

The cast becomes the narrower type of the ifNone: arg and what the return value’s context allows. For example, here the return type gets the last word, [String] instead of [Any]:

let stringContent: [String] = resource().contentAsType(ifNone: [])

(It’s one of my favorite weird-awesome features of Swift that the inferred return type changes the runtime behavior of contentAsType()!)

In fact, if there were to be a no-default version of this method — there isn’t, at least now, because it’s no more useful than as? — but if there were, it would work like this:

let stringContent: [String]? = resource().contentAsType()

That line of thinking lead me to the current name.

Correct me if I'm wrong, but AFAICT the difference between resourceWithURL and resource is that the former takes an absolute URL, and the latter is relative to the service's base URL — right?

You understand resourceWithURL() correctly, but resource(_:) is a bit more subtle. It isn’t relative, in that it doesn’t resolve ./, ../, //, etc; it just appends everything to the base URL’s path. That’s meant to prevent security holes. Suppose you let this dangerous code slip in:

let userProfile = service.resource(username)

If somebody tries to make your app leak sensitive information by making setting their username to //malicious.host, Siesta will just append that to the path: https://api.myapp.com///malicious.host, whereas relative URL resolution would switch hosts.

The only place Siesta does relative resolution is in Resource.relative(_:), which is meant to be a little harder to use by accident.

I feel like this semantic difference isn't well conveyed in the naming, and the fact that "URL" doesn't just mean "NSURL", as there's an override that takes an equivalent URL string, confuses matters even more.

What about resource(absoluteURL:) perhaps? Or maybe even resource(absolute:), since at call site the type is clear. The former is a bit more verbose than just (url:), but clearer about the intention, I think.

I like your line of thought. Yeah, these methods have been a point of confusion for multiple users, and better naming might help. Since the distinction between the two methods is not absolute / relative, but rather URL / path, maybe this naming I shied away from is the right one after all:

resourceWithPath(_:)
resourceWithURL(_:)

Or:

resource(path:)
resource(url:)

@rayfix, any thoughts on this?

Thanks for the sanity check on onCompletion() and friends. You have a good point about ResourceObserver.onResourceChange(...), thought it’s a bit different because it doesn’t accept a closure, so I’m not convinced it should follow the same convention. The Cocoa way would be resourceDidChange(…).

This is one of those cases where repeating type information might be preferable after all. And the reason why is that "URL" isn't just type information here, it also actually describes the thing the property is. And "base" works to describe "URL", not as a standalone noun.

At first glance "Service.base" seems to suggest that there's a class called Base, or perhaps ServiceBase, or that there's some fundamental concept of a "base". And there isn't. There's just a base URL.

Hmmm, you’re making me lean back to baseURL after all. Just base does open the door to many a misunderstanding.

from siesta.

radex avatar radex commented on May 18, 2024

let stringContent: [String]? = resource().contentAsType()

But that sounds weird too. "content as type"… yes, what type? Where's the type parameter? To my eyes, it looks incomplete.

What about: resource().typedContent()?

It doesn't say much more, but there's no confusion. It just says "this is a typed version of the content", and you just about have to assume the type will be taken from the context this is passed to.

You're right that when assigning to a variable it's not very useful, since as? does the job. But it could be useful when you want to immediately pass the typed content as a parameter to some method. Then that method just supplies the type.

And I think now typedContent(ifNone: ...) works much better, naming-wise.

Since the distinction between the two methods is not absolute / relative, but rather URL / path, maybe this naming I shied away from is the right one after all:

resourceWithPath(:)
resourceWithURL(
:)

Okay, so I did get the point, just used the wrong words to describe the difference.

But I would argue that "path" vs "url" is just as confusing. At least in the context of our community. All fault is on Cocoa, where people sort of learned that "path" is nothing else than a NSString version of NSURL (caveats apply).

My reaction to this is "oh, so if I want to pass a string I use resource(path:), and when I want to pass NSURL, I use resource(url:)". Which misses the point.

Obviously something super verbose like "resourceRelativeToBase" would be undesirable as well...

Ideas?

You have a good point about ResourceObserver.onResourceChange(...), thought it’s a bit different because it doesn’t accept a closure, so I’m not convinced it should follow the same convention. The Cocoa way would be resourceDidChange(…).

Fair point!

from siesta.

rayfix avatar rayfix commented on May 18, 2024

Thinking aloud here but what about
resource.content(or: placeholder) .

Then change the current content() to untypedContent() or rawContent()

(The or: is an idea I got from some API that Airspeed Velocity did recently. I think it looks nice and is still clear.)

For the issue:
resource(url:)
resource(path:)

You could define a simple Path type that better documents Path as something more akin to a Rails path type that gets appended to a base URL.

from siesta.

pcantrell avatar pcantrell commented on May 18, 2024

Hmm, perhaps “path” is indeed the problem. How about “endpoint” as the other term?

resource(endpoint: "/users")
resource(url: "https://other.server/dingbats")

That’s not perfectly correct, though, because of course external URLs can also be endpoints. Plus it loses the concision of the common case:

resource("/users")

What about this?

resource("/users")
resource(absoluteURL: "https://other.server/dingbats")

On the typed content question, it's best to evaluate the method name in context, I think. One common case is defining an extension to provide a new convenience accessor — here, for example, one that lets you do resource.image:

let placeholderImage = UIImage(named: "placeholder.png")

extension TypedContentAccessors {
    var image: UIImage {
        return contentAsType(ifNone: placeholderImage)
    }
}

So there’s the problem: “as what type?” Like Radek said, it sounds like there’s a parameter missing — and that’s correct, in a way, because there is an implicit type parameter.

With some of the alternatives:

extension TypedContentAccessors {
    var image: UIImage {
        return typedContent(ifNone: placeholderImage)
    }
}

This isn’t bad, but it's not perfect. It makes it sound like a resource can either have “typed content” or not, but all content is typed; it just may be of the wrong type. With the word “as” gone, it doesn’t convey in the same way the connection to Swift’s as?.

Ray’s suggestion:

extension TypedContentAccessors {
    var image: UIImage {
        return content(or: placeholderImage)
    }
}

I don’t think this conveys the type conversion nature of the operation enough. It’s a nice convention if we’re only defaulting for nil, but we’re defaulting for nil or wrong type.

Does a preposition change or more verbosity fix it? I don’t think so, but throwing it out here:

extension TypedContentAccessors {
    var image: UIImage {
        return contentOfType(ifNone: placeholderImage)
    }
}

extension TypedContentAccessors {
    var image: UIImage {
        return contentAsMatchingType(ifNone: placeholderImage)
    }
}

Here’s another set of “in the wild” comparisons, for the common case of extracting a specific model type, and using Radek’s idea of the bare inferred type converter with no default value:

if let user: User = resource.contentAsType() {
  ...
}
if let user: User = resource.typedContent() {
  ...
}
if let user: User = resource.content() {
  ...
}
if let user: User = resource.contentOfType() {
  ...
}
if let user: User = resource.contentAsMatchingType() {
  ...
}

That touch off any opinions, or new ideas?

from siesta.

radex avatar radex commented on May 18, 2024

What about this?

resource("/users")
resource(absoluteURL: "https://other.server/dingbats")

Funnily enough, that's exactly what I suggested a few comments before :) (albeit admittedly didn't make it this explicit).

So yes, +1. I'd also be willing to consider dropping the "absolute" word, and just have resource("/path") and resource(url: "http://..."). I have a sense the distinction is still perfectly clear, but I could be wrong.

This isn’t bad, but it's not perfect. It makes it sound like a resource can either have “typed content” or not, but all content is typed; it just may be of the wrong type.

Good point, haven't thought of that.

I think the reason why I'm drawn to the typedContent name is that I'm finding this an increasingly common pattern in my Swift code, to have methods that provide typed versions of untyped methods. (Generally this takes a form of extending an Apple class when some method or property was just defined as id or NSArray, despite being more specific; but also makes sense in deserialization context, like here.)

And when it starts being a pattern, seeing "foo" and "typedFoo" pairs in many places becomes a nice convention, and has a nice symmetry.


Out of these:

if let user: User = resource.contentAsType() {
if let user: User = resource.typedContent() {
if let user: User = resource.content() {
if let user: User = resource.contentOfType() {
if let user: User = resource.contentAsMatchingType() {

I still like typedContent best.

contentOfType seems like no improvement over contentAsType (perhaps even worse since there's no connection to as?).

contentAsMatchingType is really good, most descriptive of all the above, but I think less verbose name would be good in this context, because I think we should to encourage people to use the typed version whenever possible.

EDIT:

"content" would be great (and, if needed for whatever reason, "rawContent" for untyped version) if only the type was explicit or unambiguous somehow. But since it's inferred from the context, I think it would only bring confusion.

from siesta.

pcantrell avatar pcantrell commented on May 18, 2024

I'd also be willing to consider dropping the "absolute" word, and just have resource("/path") and resource(url: "http://..."). I have a sense the distinction is still perfectly clear, but I could be wrong.

Ha! That is exactly my original design! Full circle.

I also thought it was clear, but it was a point of confusion for at least a couple of different people. I’d tentatively lean toward this, then, since we both like it:

resource("/users")
resource(absoluteURL: "https://other.server/dingbats")

Ray, others, gut check on that?


I still like typedContent best.

Funny thing: that was my original name for it. I switched to contentAsType because of the helpful presence of the word “as.”

I still like these two best, and definitely agree with Radek’s analysis of the others:

if let user: User = resource.contentAsType() {
if let user: User = resource.typedContent() {

I could be talked back into typedContent if others agree it’s better.

from siesta.

rayfix avatar rayfix commented on May 18, 2024
resource("/users")
resource(absoluteURL: "https://other.server/dingbats")

Looks good to me.

if let user: User = resource.contentAsType() {
if let user: User = resource.typedContent() {

I slightly prefer typedContent. Seems just as clear if not more clear than the first one (first looks like it should have an arg) and also is less typing. :haha:

from siesta.

pcantrell avatar pcantrell commented on May 18, 2024

I’ve locally gone back to baseURL and made the migration to onSuccess() etc. Both changes look good as I actually use them in code.

I’ve also experimented with going back to typedContent(ifNone:), and I think you two have a point. Also, you make me see the value of the no-args typedContent() that works just like as? but uses type inference. Even this simple case:

if let user: User = userResource.typedContent() { … }

…is more pleasant than:

if let user = userResource.latestData.content as? User { … }

So … yeah, I think you’ve won me over. If I still like it by the end of the weekend, it shall be so.


On reflection, I’m liking the new use of “absolute” in the method name, but am on the fence about this:

resource(absoluteURL: "https://other.server/dingbats")

…versus this:

resourceWithAbsoluteURL("https://other.server/dingbats")

Like you all, I find the first one appealing, but it does clearly violate Apple’s guidelines as well as common practice. @erica, any chance I can nudge you into a gut reaction on this one? Seems like the kind of call you’re good at making.

from siesta.

radex avatar radex commented on May 18, 2024

Even this simple case:

if let user: User = userResource.typedContent() { … }
…is more pleasant than:
if let user = userResource.latestData.content as? User { … }

I'm not so sure about this use case (I tend to avoid explicit type declarations inside methods like that), but one that I find compelling is when the type can be inferred from a method you're passing typedContent() to.

On reflection, I’m liking the new use of “absolute” in the method name, but am on the fence about this:

resource(absoluteURL: "https://other.server/dingbats")
…versus this:
resourceWithAbsoluteURL("https://other.server/dingbats")

Like you all, I find the first one appealing, but it does clearly violate Apple’s guidelines as well as common practice.

The common practice is based on the fact that Objective-C doesn't really have the concept of argument labels the same way Swift does, rather, it's all part of a single the method name. So you have a common pattern of fooWithBar(arg), but I would argue this is based on ObjC's limitations/design, rather than a preferred syntax in Swift.

Notice how Objc->Swift bridging does a better job at translating init names.

(instancetype) initWithName: (NSString*) name

becomes:

init(name: String)

Because it simply makes more sense. "Name" describes the parameter, it's not really a part of the method name itself.


As noted before, I also disagree with you saying that this "clearly" violates guidelines. Again:

In other words, usually:

First parameters to methods and functions should not have required argument labels

Note "usually". The guidelines note a few exceptions, but I would argue the list is incomplete, and this is a good use case where it makes more sense to use the explicit argument label rather than force it to be part of the name.

Also, don't forget about:

Omit Needless Words. Every word in a name should convey salient information at the use site.

Why say "with" in the name, when separating "absoluteURL" into a param name conveys the same semantics (and arguably, better)…

from siesta.

pcantrell avatar pcantrell commented on May 18, 2024

I'm not so sure about this use case (I tend to avoid explicit type declarations inside methods like that), but one that I find compelling is when the type can be inferred from a method you're passing typedContent() to.

Sure, you could also do this nice refactoring:

showUser(userResource.typedContent())

You’d then have to make showUser()’s arg optional:

func showUser(user: User?) { … }

…but that’s probably the right thing anyway, since “show that there is no user” is also a valid & important operation.

And I still do like typedContent just fine after sleeping on it and don’t have a strong opinion either way, so I’ll defer to the judgement of R* on this.


OK, Radek, I think you’ve convinced me on resource(absoluteURL:). I like it better that way as a matter of personal taste, and I agree completely with all of your reasoning.

I say “clearly violates” because after that “usually” you mention, the Apple guidelines then say, “There are only a few exceptions,” and this is not among those they list. However … I don’t see any examples in the guidelines that clearly promote a first arg label to be part of the method name. The closest they come is this:

func addObserver(_ observer: NSObject, forKeyPath path: String)

…but in this case, addObserver seems like an operation that’s distinct from other kinds of adding, and it makes sense for “observer” to be in the base name. Is that line of thought inconsistent with the guidelines? Well, they do say this:

Methods can share a base name when they share the same basic meaning but operate on different types

That’s “can” and not “should,” but it does suggest we’re not out of line thinking that method base names are about the method’s role, and not its first argument.

from siesta.

erica avatar erica commented on May 18, 2024

I'm not ignoring you as style is pretty much at the top of my interest list right now, it's just that I'm dealing with one ridiculous family drama after another. Last kid goes back to school on Thursday.

I'm having big issues with the current minimized APIs in that you must quick-look the apis often to see the name of the parameter label, which is not a reasonable thing to do when you're trying to read code.

Apple's guidelines are squooshing together usecases: first, is implementation, where you see the entire signature (their primary design goal) but they're not properly serving the more common use-case: reading and reviewing. Remember: code is written once and read often, so readability should focus on serving the latter over the former and I'm not sure their guidance s proper here.

There's the older style API guides that would suggest:

resourceWithAbsoluteURL("https://other.server/dingbats https://other.server/dingbats")

It's clear, its immediately understandable, and it's a mouthful. vs this which is not

resource("https://other.server/dingbats https://other.server/dingbats")

This would be way better but it violates style with its initial label, use of type, etc.

resource(absoluteURL: "https://other.server/dingbats https://other.server/dingbats")

Guidance says:

  • Omit Needless Words: With is a needless word
  • Describe a role ("resource") rather than its type ("URL")
  • Use a noun phrase (this doesn't mutate and it's functional, returning something)
  • Avoid ambiguity (use "absolute" to differentiate from relative or whatever)

so I suppose the "suggested" API would be:

absoluteResource("http://other.server/dingbats http://other.server/dingbats")

Which I don't love

-- E

On Jan 10, 2016, at 10:40 AM, Paul Cantrell [email protected] wrote:

I'm not so sure about this use case (I tend to avoid explicit type declarations inside methods like that), but one that I find compelling is when the type can be inferred from a method you're passing typedContent() to.

Sure, you could also do this nice refactoring:

showUser(userResource.typedContent())
You’d then have to make showUser()’s arg optional:

func showUser(user: User?) { … }
…but that’s probably the right thing anyway, since “show that there is no user” is also a valid & important operation.

And I still do like typedContent just fine after sleeping on it and don’t have a strong opinion either way, so I’ll defer to the judgement of R* on this.

OK, Radek, I think you’ve convinced me on resource(absoluteURL:). I like it better that way as a matter of personal taste, and I agree completely with all of your reasoning.

I say “clearly violates” because after that “usually” you mention, the Apple guidelines then say, “There are only a few exceptions,” and this is not among those they list. However … I don’t see any examples in the guidelines that clearly promote a first arg label to be part of the method name. The closest they come is this:

func addObserver(_ observer: NSObject, forKeyPath path: String)

…but in this case, addObserver seems like an operation that’s distinct from other kinds of adding, and it makes sense for “observer” to be in the base name. Is that line of thought inconsistent with the guidelines? Well, they do say this:

Methods can share a base name when they share the same basic meaning but operate on different types

That’s “can” and not “should,” but it does suggest we’re not out of line thinking that method base names are about the method’s role, and not its first argument.


Reply to this email directly or view it on GitHub #15 (comment).

from siesta.

pcantrell avatar pcantrell commented on May 18, 2024

Thanks, Erica. I didn’t think you were ignoring us! Certainly nobody’s obliged to respond to a random Github mention, especially on the weekend, so I’m delighted to have you jump in at all.


To give some background, because I know this is a long thread to jump into: there are several different resource(...) variants going on. They all are essentially the same operation underneath and return the same thing, but take different parameters.

One variant takes a subpath of a service’s base URL; the other takes a fully independent URL, and that latter one comes in two flavors that accept String and NSURL. So there’s both a “path vs. URL” distinction and an “under the base URL vs. absolutely nothing to do with base URL” distinction.

I lean (as I think do Ray & Radek) toward making the more common “subpath of base URL” variant be unlabeled, because with the strings you’d actually be passing to that, it really is pretty clear at the call site:

myAPI.resource("/dingbats")

…whereas more explicit labels like this get ugly, especially given that they’re such a common case:

myAPI.resource(subpathOfBaseURL: "/dingbats")

(end of background)


I totally agree that this is not good:

absoluteResource("http://other.server/dingbats")

…because, among other reasons, there’s no such thing as an “absolute resource.” Every resource has a fully resolved URL; the only difference between all these resource(...) methods is how that URL is derived. The word “absolute” modifies the parameter, not the result.

When you say that “this would be way better:”

resource(absoluteURL: "https://other.server/dingbats")

…you are in agreement with everyone else in this thread. It passes my own gut check, and three other developers whose opinions I respect a lot say it looks good to them. That’s enough for me to go against the Apple guidelines.

If we do get a chance to comment on the API guidelines on swift-evolution, then we can make our stand there! Get your torches and pitchforks ready, I guess?

from siesta.

erica avatar erica commented on May 18, 2024

I think what we need to do is start writing some proper API guidelines that aren't limited to or just a response to the ones currently on offer (set up a repo?), start hitting them with real world examples like siesta, and then submit our guidelines once Apple opens up the API guides to public review.

-- E

On Jan 10, 2016, at 2:32 PM, Paul Cantrell [email protected] wrote:

Thanks, Erica. I didn’t think you were ignoring us! Certainly nobody’s obliged to respond to a random Github mention, especially on the weekend, so I’m delighted to have you jump in at all.

To give some background, because I know this is a long thread to jump into: there are several different resource(...) variants going on. They all are essentially the same operation underneath and return the same thing, but take different parameters.

One variant takes a subpath of a service’s base URL; the other takes a fully independent URL, and that latter one comes in two flavors that accept String and NSURL. So there’s both a “path vs. URL” distinction and an “under the base URL vs. absolutely nothing to do with base URL” distinction.

I lean (as I think do Ray & Radek) toward making the more common “subpath of base URL” variant be unlabeled, because with the strings you’d actually be passing to that, it really is pretty clear at the call site:

myAPI.resource("/dingbats")
…whereas more explicit labels like this get ugly, especially given that they’re such a common case:

myAPI.resource(subpathOfBaseURL: "/dingbats")
(end of background)

I totally agree that this is not good:

absoluteResource("http://other.server/dingbats")
…because, among other reasons, there’s no such thing as an “absolute resource.” Every resource has a fully resolved URL; the only difference between all these resource(...) methods is how that URL is derived. The word “absolute” modifies the parameter, not the result.

When you say that “this would be way better:”

resource(absoluteURL: "https://other.server/dingbats")
…you are in agreement with everyone else in this thread. It passes my own gut check, and three other developers whose opinions I respect a lot say it looks good to them. That’s enough for me to go against the Apple guidelines.

If we do get a chance to comment on the API guidelines on swift-evolution, then we can make our stand there! Get your torches and pitchforks ready, I guess?


Reply to this email directly or view it on GitHub #15 (comment).

from siesta.

pcantrell avatar pcantrell commented on May 18, 2024

If I you set up the repo, I’ll definitely drop in my $0.02!

FWIW, I really like the new guidelines in a great many respects. They’re just too prescriptive given the narrow range of cases they actually consider — particularly around part-of-speech questions and parameter naming.

from siesta.

radex avatar radex commented on May 18, 2024

Awesome! Great discussion, and thanks @erica for chiming in!

FWIW, I really like the new guidelines in a great many respects. They’re just too prescriptive given the narrow range of cases they actually consider — particularly around part-of-speech questions and parameter naming.

Same. I think the guidelines have room for refinements, and if there's something we can do as a community to show examples where seemingly the best option goes against the guidelines, then we should do it.

Let's just not get hung up on following the guidelines 100%. It's "guidelines", not "the law" — the word itself suggests that it describes the 90% or maybe 95% cases, but there's always room for good judgement for the edge cases. I'm more interested in the spirit of the guidelines, not the letter.

from siesta.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.