Philosophy of generated code

[tomlokhorst]

This is a discussion, rather than a bug or feature request.

I think we should try to generate "readable" code (defined later on).

In principle, users shouldn't have to look at the generated code. R.* can be used through autocomplete.
However, in my opion there are two types of people who do read the generated code:

1. Someone debugging an issue. When something goes wrong, e.g. if an unexpected image shows up when using R.image.someImage. A user might want to look "under the hood".
2. Someone new to R.swift. When inheriting a code base that uses R.swift, a user may look at the generated to code, to figure out what's going on.

For these two use cases, readability of code is very important. So as not to scare someone off.
In particular, I think the following should be non-goals:

1. Don't generate as short code as possible. Clarity is more important than brevity
2. Don't use the generated code as an example or test of R.swift.Library extensions.

For example, instead of generating this code:

static func textCell(_: Void) -> UINib {
  return UINib(resource: R.nib.textCell)
}

This more straightforward code should be generated instead:

static func textCell(_: Void) -> UINib {
  return UINib(nibName: "TextCell", bundle: nil)
}

The first piece of code, using the UINib(resource:) initializer uses 2 or 3 indirections.
When trying to figure out how R.swift works, or what code is actually run, the second piece of code is simpler.

[mac-cain13]

I agree with the approach you propose here. Until now the generated code only followed principles like:

• Always compile (rather skip an item than generate corrupt code for it)
• Never crash (no use of ! for example)

But nothing on readability or clarity, I feel it's nice to follow your proposal.

One question/concern I have is that this will result in not using the R.swift.Library methods, but "plain" Swift code instead. This makes the library methods harder to discover (minor issue) and will mean that fixing an issue in the library, like this one fixing incorrect use of ?., will not "automatically" fix the generated code by the compiler complaining about changed types. This could result in having to fix things in multiple places.

What do you think?

[tomlokhorst]

Yes, writing "plain Swift" is a bit more cumbersome than using the functions from R.swift.Library. This is of course the whole reason why these convenience initializers were created! 😄

However, I think the benefit of readability for the hundreds (tens? thousands?) of users reading the code, outweighs the (slight) cost of writing the code for maintainers of R.swift.

In my opinion the price is worth it.

[tomlokhorst]

Also, whatever the final result of chosen principles is:

These should be written down in a document somewhere, so that future R.swift contributors can refer to these.

[mac-cain13]

Agreed, let's from here on go for clarity.

I think we should create a Principles.md or CodingGuidelines.md in the root of the project. Stating these principles and linking to it from the Readme.

[swiftcrossing]

I think these are very interesting points. There is one thing that I am unclear about regarding how the first proposed principle would be carried out in practice.

Would you say that "Don't generate as short code as possible." is equivalent to something like "Use native calls where ever possible." or "Don't have deeply nested layers of function calls for something that could be written in a single function clearly."?

My concern is that there is an unclear line between brevity and clarity and if your are doing something complex over and over again, it might be more clear to throw that logic in a well names (well commented) function rather than repeat the logic in all places it's needed. Unfortunately, this line is obviously not well defined, so in order to enforce a rule like this, we might just have to draw a hard line somewhere, whether that be something like I mentioned above or something new.

After thinking about it, for the Localizable.strings implementation, I was originally planning on putting the NSLocalizedString() call in a function that would be called whenever necessary, but the alternative that you proposed seems like a much nicer solution in terms of readability, even in the case of printf style strings. I guess the end result would look something like this:

static let projectKey1 = NSLocalizedString("project.key.1", comment: "")
static func projectKey2(value1: String, _ value2: Int) {
    let format = NSLocalizedString("project.key.2", comment: "")
    return String(format: format, value1, value2)
}

[tomlokhorst]

Perhaps another way to describe this would be: few (or zero?) indirections

Your example is great!
I can immediately see what the code does (because I already know the NSLocalizedString function).
If the code were to go through a function call (to factor out the , comment: "") part), I'd have to also read that function to see what that code does.

To quote Spock:
The needs of the many (people who only read a single function), outweigh the needs of the few (people who read the entire file, and see some repetition).

[yonaskolb]

I'm all for this new style. At the moment having multiple items in auto complete, one with name and one with name() is cumbersome. Also would this remove the need for (_:Void) or is that for another reason?