Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefer where {T} to where T? #69

Open
nickrobinson251 opened this issue Sep 21, 2020 · 4 comments
Open

Prefer where {T} to where T? #69

nickrobinson251 opened this issue Sep 21, 2020 · 4 comments

Comments

@nickrobinson251
Copy link
Contributor

nickrobinson251 commented Sep 21, 2020

As far as i can am aware, BlueStyle has no opinion on this.

even the example code in this style guide itself is inconsistent, sometimes using where {T} and othertimes where T (or where {T <: Int} and othertimes where T <: Int).

We could stick with having no preference, however, for the sake of running a formatter (domluna/JuliaFormatter.jl#283, #7 ), what should a formatter do?

We have four options (to my mind, anyway)

  1. always prefer braces: where {T} to where T
  2. always prefer NO braces: where T to where {T} (when valid)
  3. sometimes prefer NO braces (in a subset of cases of the valid cases, based on some rule)
  4. do nothing

Option (4) is "do nothing". This isn't a great option for 2 reasons: (1) stylistically, we probably want a code-base to be consistent on this, it's one thing to say "either is fine", and another to say "both is fine". (2) effort-wise, the formatter can/does already fix these cases, so i see no reason to add functionality to leave a code-base less consistent.

I see no reason to come up with a subtle rule here (we've survived without it so far), so let's not do option (3), which is the most complicated.

This leaves options (1) and (2). Since we have no strong preference here (based on the current style guide), i say we stick with the decision made in domluna/JuliaFormatter.jl#53 and go with option (1) -- it's the more common pattern in Julia Base, and it's what JuliaFormatter does right now -- so for the sake of the formatter, prefer where {T} to where T.

@iamed2
Copy link
Collaborator

iamed2 commented Sep 22, 2020

My personal preference when writing code is no braces for long-form functions with a single typevar, and braces otherwise. But for the purposes of this decision, I vote Option (1).

Also Option (2) would lead to some cases I consider unfortunate:

Option 1:

function foo(::Type{A}) where {T <: Integer, A <: AbstractArray{T}}
    return A
end

Option 2:

function foo(::Type{A}) where A <: AbstractArray{T} where T <: Integer
    return A
end

I don't know anyone who uses the latter form in that scenario.

@omus
Copy link
Contributor

omus commented Sep 22, 2020

I typically use Option (3) which is to prefer braces but exclude braces when only defining a single type parameter.

@nickrobinson251
Copy link
Contributor Author

nickrobinson251 commented Sep 22, 2020

prefer braces but exclude braces when only defining a single type parameter

To clarify this would be:

# Yes
f(x::T) where T = ...

# No
f(x::T) where {T} = ...
# Yes
f(x::T) where T<:Integer = ...

# No
f(x::T) where {T<:Integer} = ...
# Yes
f(x::T, y::S) where {T,S} = ...

# No
f(x::T, y::S) where T where S = ...

(where the guidance is the same for both short-form and long-form functions)
I could be onboard with that... i think that's how i tend to write things :)

@omus
Copy link
Contributor

omus commented Sep 23, 2020

I wouldn't say using curly braces all the time is wrong just not preferred:

# Yes
f(x::T) where T = ...

# Ok
f(x::T) where {T} = ...
# Yes
f(x::T) where T<:Integer = ...

# Ok
f(x::T) where {T<:Integer} = ...
# Yes
f(x::T, y::S) where {T,S} = ...

# No
f(x::T, y::S) where T where S = ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants