Rewrite joinWithSeparator's zero count separator if/else statement with an early-return if check

[Nirma]

Since the majority of the time this function is called, it is called with the intention of inserting a separator
the case of the separator not existing is less common, therefore it might make intent clearer if the if/else check in this code was rewritten with a guard statement.

[gribozavr]

I find the double-negative here very hard to read:

guard separatorSize != 0 else

[Nirma]

@gribozavr Sorry about that, I changed it to:

guard separatorSize > 0 else

[gribozavr]

It still forces a negation onto the condition. I don't see a reason to use guard here. We don't need to bind any variables in the outer scope.

[Nirma]

There is an example of this in stdlib/public/core/Reflection.swift

https://github.com/apple/swift/blob/e249fd680ebc4c7835370dedb57ceab2b0c73655/stdlib/public/core/Reflection.swift#L179

Where there is a similar use of a guard with no binding variable

guard maxItemCounter > 0 else { return }

[gribozavr]

I'm not a fan of that line either, FWIW, but that use is more readable, since it immediately returns. In the case of this PR, we are doing non-trivial work in the else, so when reading the downstream code, the condition under which the code executes is likely lost from sight.

[Nirma]

@gribozavr I see your point, my original intention was to simplify the the downstream code and take it out of an if/else.
The intention was that the less common case of having an empty string as a separator is dealt with early and we do a simple cleanup & return in the else clause of the guard statement.

By doing that the code handling the more common case of having a non-empty string separator is less nested.

Most of the time if someone wants to join an array of strings with no separator I would imagine that they would reach for reduce instead of this function.

[gribozavr]

If you want to add an early return and de-indent the function, that's good, but we can also use if for that.

[Nirma]

@gribozavr Yes the goal was to de-indent the function a bit and replace the if-else with an early return be it guard or if but now it makes more sense to use an if instead.

I made the change and squashed the commits into one, how does it look now?

Thanks for all the feedback!

[gribozavr]

@swift-ci Please smoke test

[gribozavr]

LGTM now, thank you!

[Nirma]

@gribozavr The Smoke test failure seems unrelated, what do you think?