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

Unless is complex to read #190

Closed
janpieper opened this issue Aug 29, 2024 · 5 comments
Closed

Unless is complex to read #190

janpieper opened this issue Aug 29, 2024 · 5 comments

Comments

@janpieper
Copy link

janpieper commented Aug 29, 2024

Versions

Elixir

$ elixir --version
Erlang/OTP 27 [erts-15.0.1] [source] [64-bit] [smp:16:16] [ds:16:16:10] [async-threads:1] [jit:ns]

Elixir 1.17.2 (compiled with Erlang/OTP 27)

Styler

$ mix deps | grep locked | grep styler
locked at 1.0.0 (styler) 4ba5bc40

Example Input

if env != :test do
  # ...
end

Current Behaviour

unless env == :test do
  # ...
end

Problem

We're about to upgrade styler to 1.x and I was shocked after checking what have changed after running the formatter 😲

I know that Elixir has if and its negated version unless, but I really don't get why unless env == :test should be more readable than if env != :test 🤔 Just because it is there doesn't mean we need to use it.

When reading the code I first see env == :test and to know whether this is true or not I also need to look whether it is an if or unless clause. So, I always need to think about two things instead of just one when simply using a if-clause.

Maybe that's just because of how my brain works, especially because I never used unless before, but I would really like to know whether using unless is really better than having an if-clause with a negation.

@novaugust
Copy link
Contributor

Cheers Jan, thanks for caring enough to open an issue =)

Sorry you were shocked! It is in the release notes:

if/unless: invert if and unless with != or !==, like we do for ! and not #132

As to your question:

I would really like to know whether using unless is really better than having an if-clause with a negation.

Who's to say! Just a style thing. Consistency, at least, is better than inconsistency, and Styler was already rewriting some simple negations to unless statements. Anyways, we take no offense to the folks who fork styler to make it do things more to their own personal tastes

I'm open to changing features based on feedback - so please don't feel discouraged from opening more issues - but typically am only looking to add them, as opposed to remove them. The things Styler's doing, it's doing because we want it to. Like it says in the readme:

Ultimately Styler is @adobe's internal tool that we're happy to share with the world. We're delighted if you like it as is, and just as excited if it's a starting point for you to make something even better for yourself.

Have a good one =)

@ypconstante
Copy link

Based on this PR is looks like Elixir is planning on soft deprecating unless

@novaugust
Copy link
Contributor

ooo thanks @ypconstante! now there's a solid reason to nuke our unless rewrites :D

@novaugust
Copy link
Contributor

if the core team goes ahead with that i'll definitely be updating styler to remove all unless's, likely a 1.1 release

@novaugust
Copy link
Contributor

released 1.1, which removes unless from codebases. thanks for bringing this up @janpieper & @ypconstante 🙏

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