Confusion when reading large open source library codebases

Perhaps we have different definitions, but as I understand it “cargo cult” means copying what you see other people doing without understanding why.

And I would draw a significant distinction between “I saw other code structured with extensions, so I’ll do the same even though I don’t understand it,” and “Wow, that code looks great when it’s organized into extensions, I’m going to adopt the same style myself because I like it a lot!”

The first one is cargo-culting, the second one is…discovering something that you personally like.

4 Likes

I would broaden the definition, but I guess it's more or less the same:
It boils down to keep doing things without ever questioning them.

I'd say unless you can add another "because", it is still cargo cult — but that is not bad per se.
There's nothing wrong with code that looks great; there are just other aspects which I consider more important. One example for this is when people start treating their source like ASCII-art and use whitespace extensively to line things up, making it hard to correct names or add new lines.

Like in a true religion, a cargo cult starts to become nasty when members try to force their believes onto others; in a mild form, this already happens with protocol conformances, which are so common that you sometimes have to justify when you refuse to put them into extensions - whereas I never have seen a proper justification to follow that practice.

“I like it better that way” is a perfectly valid justification for a style choice.

3 Likes

"I like it better that way" is a perfectly valid justification for me — it is a terrible justification for someone who has good reasons to prefer another style. ;-)

The justification is that it makes it easy to see at a glance which methods are related to which protocol. For a type that conforms to many protocols, this can make a big difference in readability.

It also gives you that extension’s scope as a tool to use when organizing code. Got a helper method/property that’s only used in that conformance? Put it in the extension and make it private. Now it’s immediately clear to the reader that it’s only being used in that conformance.

3 Likes

Extensions can be used to structure code — but they are not particular well suited to do so.
You have to do everything by yourself, without support from neither standard tooling nor the language itself, and there is no way to guarantee that

  • the "protocol extension" only contains code which is relevant for the protocol
  • all code relevant for that protocol is contained in the extension

It is very easy to mix things up, and wrong structure does more harm then no structure at all.
Maybe in the future things will change, but until then, comments have some big advantages over extensions: They can't give you that false sense of security — and they have tooling support, as Xcode will show structuring comments in the method menu.

That was the driving motivation for adding fileprivate and adding stronger restrictions to private (SE-0025) — but I have sad news for you: SE-0169, which shredded that guarantee, turning it into another source for false sense of security.
The irony in this story is that SE-0169 was motivated by the "cult" of using conformances to structure code… therefor, I think it's valid to consider that ritual as harmful, as it is responsible for a big complication in the system of access rights.

Still, I don't say you should stop using "same-file-extensions" — I'm fine with that.
However, I wish all people would think more about pros and cons themselves instead of repeating claims from blogposts they read.
Above all, everyone should be free to decide that it's more reasonable to put their methods in the main declaration, without having to justify this preference (to be clear: I'm not blaming anyone here for suppression — but the cult is much bigger ;-).


1 Like

Neither of these is even desirable to guarantee.

There will often be members semantically related to protocol requirements, sometimes refactored out of them, that are not themselves requirements. It makes sense to place them in proximity to the requirements to which they relate.

There can be members that are requirements of more than one protocol which happen to compose well together. There can be members that happen to be requirements of some protocol, but are semantically so related to other members of the type that have nothing to do with the protocol, and which would exist on the type whether the protocol conformance were implemented or not. It can make sense to organize these members elsewhere, apart from other protocol requirements.

No, the reason one should use extensions to structure protocol conformances has to do with neither of these points.

Rather, this strategy allows you to build up a series of conformances to a protocol hierarchy in layers, and to have the completeness of your conformance at each step checked by the compiler. This is not just a nice-to-have, but rather it is essential to avoid creating infinite recursion inadvertently.

Consider the following toy protocol hierarchy:

// written freehand; pardon any typos
protocol A {
  var isFrobnicated: Bool { get }
}
protocol B : A { }
protocol C : B {
  func frobnicated() -> Int
}
extension C {
  func frobnicated() -> Int { isFrobnicated ? 42 : 0 }
}

Now consider this conformance:

struct S : C {
  var isFrobnicated: Bool { frobnicated() != 0 }
}

Yikes! Now you've got an infinite recursion. What went wrong?

Default implementations of protocol requirements are built from requirements that aren't defaulted. (It couldn't be otherwise, if you think about it.) You always run the risk of infinite recursion if you implement a non-defaulted protocol requirement by calling a default implementation provided in the same or a more refined protocol in the hierarchy. Even if the default implementation today doesn't call the requirement you're implementing recursively, a future version could.

So how do we decrease the risk of making this mistake unintentionally?

In my experience, it's usually pretty obvious when a requirement of A calls a default implementation of another requirement of A. Possibly in part because, when you're working on it, these requirements are all at the forefront of your mind.

It's much more difficult to reason about requirements that have default implementations elsewhere in the protocol hierarchy; it can easily slip your mind which members you've implemented yourself and which ones you didn't, particularly when requirements differ only by argument or return type. Sometimes, with this kind of overloading, you may think you're calling a member required by a less refined protocol which you've already implemented, but mistakenly call a member required by a more refined protocol which you haven't implemented, but which has a default implementation!

This is where it helps greatly to build up the conformance layer by layer. If first we ensure that S has a working conformance to A without trying to conform to B or C, and then we ensure a working conformance to B, and then to C, we can avoid this pitfall. This is because an implementation when conforming to A can't accidentally call a defaulted requirement of C while the type doesn't yet conform to C. The natural way to do this is to write extension S : A { ... } first, ensure that everything compiles and works as expected, then proceed to extension S : B { ... }, and so on.

Do examples of such unintentional infinite recursion actually happen? Yes! Does this strategy of building up conformances in layers actually help? Yes--only by adhering to this stringently was I able to avoid accidentally causing infinite recursion in implementing certain DoubleWidth functions (now sadly relegated to a prototype instead of part of the standard library).

Now, could you take this same strategy but in the end lump everything together again without using a series of separate extensions? Sure, but that's depriving your reader of the ability to follow along in the building-up of the type in the same way that helped you write the code.

8 Likes

:+1: … and as extensions can't add stored properties, it's also often impossible have a clean separation without extra boilerplate.

That's a pattern I like as well — but afaics it's more common at library/framework level than in application code.

However, extensions are also used in other situations:

… and I agree with @joaqo that this not a good practice.
A real-world example for the kind of misuse I'm talking about looks like this:

class MyTableViewDataSource {
  var entries: [Entry] = []
  //… lots of code
}

extension MyTableViewDataSource: UITableViewDataSource {
  func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
     return entries.count
  }
  func tableView(_ tableView: UITableView, estimatedHeightForRowAt indexPath: IndexPath) -> CGFloat {
    return 50
  }
 //…. lots of code
}

extension MyTableViewDataSource: UITableViewDelegate {
  func tableView(_ tableView: UITableView, canEditRowAt indexPath: IndexPath) -> Bool {
    return true
  }
//…. more code
}

It's fine to not like a particular style choice—that's what style is all about—but to label it as "misuse" or dismissing it as "cult"-ish or a "cargo cult" is pretty hyperbolic and a bit insulting to folks who do use that style.

The pattern that you're labeling as misuse here can be found throughout the source of Swift's own standard library. Are you suggesting that the Swift team is misusing or cargo culting their own language?

5 Likes

You can’t even do this if you want a set-like collection; both SetAlgebra and Collection define isEmpty.

1 Like

Please excuse me if I chose inappropriate words — but please also don't assume bad intentions:
Neither did I say that every use of extensions is "cultish" (and/or misuse), nor did I use the word in a discriminative way.
The dictionary offers harmless interpretations as well.
cult (noun) someone or something that has become very popular with a particular group of people
misuse (noun) an occasion when something is used in an unsuitable way or in a way that was not intended

I have little doubt that using conformance extensions is quite popular, and while I guess extensions originally had a different intention than today, I think nearly everything can be used in a wrong way.

Usually, I try hard to answer every direct question — but this kind of rhetoric figure appears too aggressive to me; let's not try to construct insults.
However, I'm convinced that the Swift team would agree that the code in my example needs fixing because it makes bad use of extensions.

You state that placing the UITableView protocol conformances in extensions is a misuse of protocols but I don't see where you explain why. I'm sure there are tens of thousands of instances of these extensions in app store apps.

For me having extensions for protocol conformance organizes the code and facilitates copy/paste of code from one class to another. They serve as templates in new code.

Using extensions to organize code is different from the use of extensions to add code to classes where one doesn't control the source code of the original class. I'll agree that it could be confusing to use extensions simply to organize code for a class that is all in one source file. Solving that would require a new language construct that allowed you to organize the code in a given source file. In the end whatever that thing would be would be an awful lot like the existing extensions.

Using comments to solve this problem may be an age old method but it seems an awful lot of developers believe that extensions are a better way. We're in an era where comments have gone the way of the dodo anyway.

No, actually that thought originates from the imagination of someone else: I gave a very specific example of harmful usage of extensions, but didn't write about UITableView helpers (or other protocols) in general.
It might well be possible that most same-file extensions could be called misuse, but I don't think it's sensible to discuss about the original intentions for that feature and the history of that custom.

As for the "why", this is explained here:
Confusion when reading large open source library codebases (with that in your mind, just look at the example very carefully — trust me, you wouldn't want that code in one of your projects ;-)

That is my key point: It's a belief that "it's better to put every protocol conformance into separate extensions". It's not a fact backed by evidence, and so it shouldn't be treated as such.
My belief is that comments are still very valuable (at least the headerdoc-flavor), but although I think there is some pretty compelling advantage, I don't feel offended by different opinions.

Maybe it was the lack of context that led to confusion, since you only said "here's an example of misuse" and posted a code snippet, so other readers like myself attempted to guess which patterns in that snippet were the alleged misuse. To help us better understand, could you be more specific about what in that code snippet you consider to be misuse? Are there concrete ways that it could be changed that still use extension-based organization that removes the misuse?

4 Likes

That's exactly what's happening in the example — and despite this paragraph, people seem to be blind to see that implementations reside in the "wrong" extensions.

You could simply put all UITableViewDatasource methods in the UITableViewDatasource extension, and the UITableViewDelegate methods in the UITableViewDelegate extension to remove the misuse that is (hopefully) undisputed — but I consider the pattern in question to be universally harmful, so moving code around only fights the symptoms, not the cause.
The question itself is somewhat funny, as it shows the strong desire to proselyte (I actually had to look this up — although I've often seen "evangelize" in a technical context, the dictionary links that verb directly to religion), whereas my point is that it's ok not to use unconditional conformance extensions.
Afaik, there has never been a single serious analysis or discussion about the effects of that custom, and actually, it might already be too late for a proper trade-off.

Thank you for pointing this out. I had completely forgotten about SE-0169.

1 Like

It's wrong to call people “blind” when it was just a poor example because you need to already be very familiar with UITableView, or chase down the documentation, to understand that the methods are in the wrong extensions. And calling people cultists (cargo or otherwise), proselytes, following rituals, just “repeating claims from blog posts” etc. is both insulting and a bad way to convince anyone of anything.

3 Likes

Wait what? The misuse in that example is that some of the methods are in the wrong protocol extension? I guess I missed that first time around. The mistake in that code is using two protocol extensions; only one is needed. For UITableView and similar I always use a single protocol extension that adopts both protocols and would never run into the misuse you describe. Your proposed solution of using comments rather than extensions certainly won't solve the misuse you describe. As everyone knows:

// Gur pbzcvyre qbrfa'g purpx fcryyva be tenzzne be pbeerpgarff va pbzzragf.

You quote SE-0025 and SE-0169 but your opinions of them seem backwards of everyone else's. SE-0169 was an improvement of SE-0025. It mentions that "Swift encourages developers to use extensions as a logical grouping mechanism..." And that's what everyone does.

There's nothing wrong with having pet peeves. I certainly do. But on this issue I think you're in a group of very nearly one.

Actually, no extension is needed in this scenario at all ;-)

I'm pretty sure even many members of the Core Team would agree with me when I say that SE-0025 in combination with SE-0169 is a regression compared to what we had before — it's just bad for the morale to concede that compatibility won over expressiveness and elegance ¯_(ツ)_/¯.
The new private solves no real problems, but it was a significant increase in complexity (I would really appreciate if no one would launch another try to turn an opinion into a case of lese majesty… :laughing:).

That's hard to tell — but there are definitely others who are skeptical towards the unnecessary use of extensions, and there are also plausible reasons for them to stay silent:
I'm sure that the pattern in question isn't popular because of facts, but rather because of beliefs which are propagated by powerful influencers.
The prospects of success in dismantling beliefs with arguments are low, and arguing against a larger clique is tough business…
Note how few tried to present solid arguments for the use of extensions, and how many grasp for oblique rhetoric to fight opinions that aren't their own.

More insults and weird conspiracy theories. I suspect nobody is presenting “solid arguments for the use of extensions” because it's off-topic for the thread. The original question about extensions was well-answered a couple of weeks ago here:

I would suggest starting a new thread if you want to try to convince people to stop writing code in a style you don't prefer.

Terms of Service

Privacy Policy

Cookie Policy