Multi line ternaries should be allowed in our style guide. #188
nickkaczmarek
started this conversation in
General
Replies: 1 comment
-
I think we landed on making multi line ternaries allowed but saying that they should be avoided if the use case doesn't really call for them ie don't get too complex. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
We talked a lot about this on slack so I'm going to copy that out and paste it here for clarity.
tl;dr making multi line ternaries allowed but saying that they should be avoided if the use case doesn't really call for them ie don't get too complex.
An example:
Discussion
Tyler Thompson 20 minutes ago
Gonna respond here so we can get concrete.
Tyler Thompson 18 minutes ago
So here's the thing. type.type == objectType (when objectType is optional) is NOT semantically equivalent to.
We've run into that problem before. The ternary relies on underlying implementation of type.type. However, what if both are nil? Did you mean to return true?
Nick Kaczmarek:jupiter: 18 minutes ago
I think this is an example of readibility, although I am not in love with this particular chunk of code. I do however find it easy enough to read. I think we could endeavor to extract the bools to make it even easier, but I think the one liner is fine for really short things but I think longer ones have a place too. Also I don't mind nested ternaries typically
Tyler Thompson 17 minutes ago
This logical statement is much easier to understand:
Intent is clear, side-effects are minimal.
Tyler Thompson 17 minutes ago
That entire thing can be returned as an expression by using a closure:
Nick Kaczmarek:jupiter: 16 minutes ago
I think it's important (in this case) to not judge the code but judge the structure
Tyler Thompson 16 minutes ago
I am trying to judge the structure. My point was on intent. Readability is first and foremost about understanding intent of the author.
Nick Kaczmarek:jupiter: 16 minutes ago
It could be written in a more concise way, but using ternaries in this way is agreeable to me. I'll have to try to contrive an example, but that might take me a little while
Tyler Thompson 15 minutes ago
The ternary does not convey intent to me. Is my conformance check correct? Or did Morgan desire different behavior? I genuinely don't know the answer to that question.
Nick Kaczmarek:jupiter: 13 minutes ago
Nick Kaczmarek:jupiter: 12 minutes ago
Maybe I'm not thinking swiftly enough, but I find the first and middle options to be nice, but with a longer version of the second, I prefer the indentation.
Nick Kaczmarek:jupiter: 12 minutes ago
super contrived, but this is about the style for me. I think the intent is clear in my example.
Morgan Zellers 12 minutes ago
So I’d prefer to stay away from analyzing the code around the ternary here. Maybe this is a bad example for the question we’re asking
Tyler Thompson 11 minutes ago
We can stay away, but I think it's a perfectly cogent example because it showed the point, multi-line ternaries opened the door to ambiguity. Your code is fine, I'm not saying you did it wrong. I'm saying that STYLE promoted code that left questions.
Nick Kaczmarek:jupiter: 11 minutes ago
That's why I try to post my super contrived example. So the discussion is less on the example and more on the usage of a ternary.
Nick Kaczmarek:jupiter: 10 minutes ago
I think I see where @Tyler.Thompson is coming from, but I'm curious if my example changes your mind in any way
Tyler Thompson 10 minutes ago
With that example my argument is that the first example inherently encourages you to keep your ternary brief.
With your middle option while I have no objection to this code, but it opens the door to abuse. It's easier for me to decide to nest them, or add WAY too much in there.
Your final example is also fine, although I actually hate it the most because it's needlessly verbose.
Nick Kaczmarek:jupiter: 10 minutes ago
it's less that I'm saying to always use a ternary and more that I think there is value in a multiline version. Perhaps your point about AVOID is apt here
Tyler Thompson 9 minutes ago
Right, that's really what I'm suggesting here. It's AVOID. If given a choice, option 1 was the best. It's clear, it's concise, it encourages readability. If people start to multiline it's not inherently evil but it probably indicates there's something else going on.
Nick Kaczmarek:jupiter: 9 minutes ago
I think what I'm getting at is that there is a place for them. I agree that they can be abused, but I want to be able to write them if it makes sense. I think we still reserve the right to say that is a bad use case, make it better but that that won't always be the case.
Nick Kaczmarek:jupiter: 8 minutes ago
Cool beans
Tyler Thompson 8 minutes ago
It's the same reason we put line limits on files. Who the hell cares how big a file is? The compiler doesn't (well..the Swift compiler kind of does...) We do it because large files usually indicate a smell or a cognitive overload
Nick Kaczmarek:jupiter: 8 minutes ago
I kinda wish we would've done this in a discussion, but the feedback loop is so tight on here.
Nick Kaczmarek:jupiter: 8 minutes ago
Agree on file size.
Tyler Thompson 8 minutes ago
multi line ternaries are the same IMO, they're not bad. They're a flag that there might be dragons.
:plusone:
1
Nick Kaczmarek:jupiter: 8 minutes ago
I might try to screen shot this and post it.
Beta Was this translation helpful? Give feedback.
All reactions