I don’t think everyone votes on these things. I was watching the mailing list activity hoping it would get shut down.
Sure, is a bit of a pain writing the same word twice but it is so much clearer. If you’re not using destructured terms in the function head for control flow, you should destructure in the body imo.
IMO such proposals are shallow, over-optimize the entirely wrong things, and miss the point of writing code in the first place: that it must be readable and immediately understandable for future programmers – including your future self who might not appreciate you being overly clever just to indeed save a few keystrokes, while he has to spend some extra time to touch base and reestablish proper understanding before making modifications.
I am finding myself constantly nodding in agreement reading @sodapopcan’s comments here. I have, very shamefully, engaged in bikeshedding even as soon as 18 months ago and at one point figured I am being stupid and just gave up then on the spot, and hopefully forever in the future. Stuff like code formatting, certain code style rules (where a lot of bikeshedding occurs), pre-commit / post-commit hooks, PR/MR checks – all these have good intentions and they must be recognized and (IMO) amplified.
At the end of the day it’s a job for money and we should optimize for standardization and productivity. Having to type out 5-10 words twice is something you’ll forget doing in 5 minutes or, as @sodapopcan said, should be a wake-up call to revisit why is it needed as often.
So you type “mm” + tab and then you have two cursors separated by a colon and a space, and you can type your key and your variable at the same time. I use it very often.
I would also like a special syntax for creating/destructuring maps as I am used to that in JS/TS, but only if it’s part of Elixir. I can’t stand having use statements in code just for syntax stuff. Syntax is not important (and I dislike Elixir’s ruby-like syntax actually). Just as libraries that make you use macros where 99% of the time plain functions would do the same work.
I destructure a lot in top of functions. I can’t forget that the . compiles to a case statement even if I’m sure the performance cost is abysmal.
Just to clarify, I also believe is a very not important thing, but if it is a little bit of an issue, is because of reading and not writing.
One thing that really surprise me (in a bad way) when coming into the elixir ecosystem, where those, many many lines long function declarations (specs + pattern matching). @sodapopcan is right, and that’s exactly what I do on my own code, I tend to do whatever it takes so the function definition stays in one line.
Having said that, I believe the current state of affair is really good: if you want it, extend the language with a sigil, and that’s that.
Interesting, I actually didn’t know that. I found this thread talking about it. Is the performance really “abysmal” though? Don’t multi-head functions compile to case as well? I know they do in Haskell but I can’t remember with Elixir.
Agreed. There are exceptions to every rule, of course, and that is if the win is big enough. Pathex for example (well, it’s doing a bit more than adding syntax in a bunch of cases). Though whenever I use or import Pathex I do in the function I’m using it in.
Amen. I have a strict rule for myself to never use multi-lined function heads. It’s never caused any problems! Credo rule pending
Hear hear. I understand if peoples’ programming style warrants it, and honestly for now I’m just happy it’s not a thing. If it does become a thing, it would be nice if it wasn’t touted as “the” way as it is in JS land. The formatter shouldn’t force it (I don’t see why it would) and hopefully Credo wouldn’t make it a default.
Agree, do it locally but don’t go beyond that, because this is more related to personal preferences and the kind of project you are working with.
Usually I don’t care much about the style the code is written in, what for me is very important is consistency. If there is no consistency, the code is automatically unreadable, be it with sigils, single line functions or other things.
From what I saw in my experience I saw this arises from people coming to the project and speculating on their personal preferences and pushing them as some kind of standard. What ends up happening with time is a split of code styles witch leads to very poor readability, think of it as a book that is written in different dialects that are getting switched randomly, all of them are in the same language per say but slightly different.
I am a strong believer that every project project should have it’s philosophy that in most cases is unique, tailored to the team, individual or business requirements and there is no silver bullet on what means good syntax.
I feel the exact same way and it was the best working on a team that was mature enough to do this. I think there are two important ingredients to making this work well. The first is to codify everything, ideally with linter rules but otherwise agree on everything else (a pretty obvious one). The second is to make it clear that nothing is set in stone and if someone really has a problem with something, they are allowed to propose change. I think this helps mitigate human nature that makes us want to rebel against being told what to do.
Of course, a lot of people will see this as a waste of time. They will argue that it’s spending time in the wrong places. The thing is, once this is “setup”, you hardly think about it again. It only goes poorly when you have people who consistently break the rules. I’ve also talked to people who actually argue that it’s a good thing to allow everyone to use their own personal style at work. I get that it can be argued that this allows people to be happier and maybe do better work but, well, that’s a whole thing I’m not gonna go on about here.
Indeed, I never start a project without introducing credo and formatter (the formatter is harder to enforce, however I try to make sure that all members use it, for example in vscode you can set format on file save). Making some official hard guidelines more strict than credo are hard, and I don’t think they can be checked by a CI, only by humans when reviewing code.
The problem with these kind of things is that they are subjective, cannot be classified and motivated entirely. The sentence “using sigils makes the code more readable” contains absolutely no argument that can be motivated, so I think this is a matter of either letting the majority decide or have the most experienced person take decisions for the entire team.
This is something I would classify as a social approach and arguably it is very important, however it is important to be executed correctly, as someone is happy with making his own bicycle, but someone else later pays when that piece cannot be extended or worked with.
Absolutely to all of that. Your team has to be good at “disagree and commit”, especially when it comes to low stakes decisions like these. This is a general skill every team should develop as it will come in handy in far more circumstances then deciding on. If someone has a real problem with something and wants to propose a change then yes, you either have to vote or appoint a single decider who can accept or shoot it down. My team voted and change proposals were very, very rare. And the reason couldn’t just be “I prefer it this way”, of course. We also paired 100% of the time so it wasn’t hard to go astray of conventions that couldn’t be caught by the linter.
I recently wrote a credo rule to enforce how assigning to socket assigns is handled. That was one area that drove me a bit nuts at my last job as everyone did it slightly differently. This convo is motivating me to get back to writing more, some are just tough. It’s just me right now so it’s not a top priority but I never know when we might need someone new and I want to have it all ready to avoid long discussions.
The business should always come before these, and I like the wording low stakes decisions, this is why a good developer that wants to be productive will never truly care much about such small details.
I never wrote any custom rules for credo, but if it is easy to do, I would definitely pursue on using it to create a few more strict rules, even though I have a feeling that this is a two sided stick and might backfire fast.
My coworker used to say something along the lines of “the low stakes decisions are the most important to make quickly.” IE, stuff that ultimately doesn’t matter but still needs a decision if you want to avoid chaos (inconsistency). I wrote a short blog post about one of our methods around this. I don’t advertise my blog much as it isn’t particularly good, but I was happy with this one.
I’ve lost patience for that crap. My standard reply nowadays is: “May I remind you that you’re at work at the moment and not everything is enjoyable? And that consistent style plus maintainability are our first priorities?”.
If people work better without computers supervising and pointing out their mistakes (linters, CI/CD) then it seems it’s time for them to leave programming.
Amusingly, little less than two years ago, my then-team never told me this was a thing so I got super stressed about every PR because even one with 15 changed lines got 5+ remarks from one team member, consistently. I thought these were deal breakers and we had a lot of back and forth on every single one. I’ll spare you the story because we also had separate meetings trying to nail down certain rules and that never got anywhere. Anyway, all that led to burnout, resentment, difficulty working together and eventually me being let go. On the exit meetings he was flabbergasted that I didn’t know that basically 90% of his remarks I was free to treat as “disagree and commit”.
One of my most weird work team misunderstandings / disconnects, by the way. I’m still remembering it every now and then.
Lol, yep, that’s how I would ideally deal with it if I’m in that position again.
Ha, sort of a double meaning for “disagree and commit.” I meant more in the “commit to the decision” sense. I had one particularly influential team member who never let these types of decisions drag on, so we never had any meetings like the one you described. As soon as it was clear the discussion was going in circles, he’d force us to vote on it, or use “Why cares the most” as outlined in my blog post.
I find this phenomenon very surprising. Was this Github, which has very clear delineation between “request changes” which can block merges, and “comment” which allows you to make suggestions without blocking the merge? I do also occasionally still get questions in a review phase to the tune of “are you alright with these changes?” because I left a non-blocking review. “No I’m not alright I’m super mad they did it that way but I didn’t block it”
Bikeshedding is such an unnecessary foot-gun with the tools at our disposable because every disagreement should start out with a clear technical concern which should block until the technical concern is resolved. If it’s a style issue then it cannot be blocking because style guidelines are already enforced with format and credo. If you think those should be changed open a new ticket with your proposal and make your case there, not on this hotfix PR.
I guess this is why good PMs can make such fabulous amounts of money without writing a line of code…
You still have to make the style guide, though. All Credo rules are optional so you have to decide which ones you’re going to go with. I don’t think there is anything wrong with this, I just think each decision should be made very quickly, like under a minute if possible. Some stuff will take a little longer, like “we need a pattern for our service objects,” but it sure as heck shouldn’t take more than 30 mins to come to a decision there (it could also be left to evolve if there is no obvious answer off the bat. There are also somethings Credo just can’t do. The only thing that comes to mind is “no single letter variables.” I’m a big fan of this rule except that x and y are totally valid in certain contexts. You could write a rule to allow x and y but then you know some dingus is just going to use them out of context. I just wish some rules could be left up to people to follow, and I have worked on a team where that was the case, unfortunately so many people attacked when they are told they have to code a certain way. A big part of it is how these rules are presented. If team mates are like, “ZOMG how could you do this!??” then I can understand why people would push back.
Anyway, sorry, this got a bit stream-of-conscious.
I wasn’t sure if you were insinuating that the . or matching would be slower. In any event, I did some benchmarks. I think my setup is fair but I’m not totally sure, I don’t really know the ins and outs of benchmarking. I got extremely varying results depending on how many of these I ran in one go (I’m talking about each benchmark, not iterations) soI won’t post results. But when running just "fn_match" and "dot" several times, they are essentially the same with "dot" winning out more often than not. But again, I have no idea if this is a fair setup.
Any insight is appreciated!
Mix.install([
{:benchee, "~> 1.0", only: :dev}
])
dot = fn %{bar: bar} ->
bar
end
match = fn foo ->
foo.bar
end
inputs = %{"map" => %{bar: "bar"}}
Benchee.run(%{
"dot" => fn map ->
for _i <- 1..1_000_000 do
dot.(map)
end
end,
"fn_match" => fn map ->
for _i <- 1..1_000_000 do
match.(map)
end
end,
"body_match" => fn map ->
for _i <- 1..1_000_000 do
%{bar: bar} = map
bar
end
end,
"case" => fn map ->
for _i <- 1..1_000_000 do
case map do
%{bar: bar} -> bar
end
end
end
}, inputs: inputs)
Sure, but “Request Changes”-s UX is kind of lame so in every Elixir job I was in we only ever used comments. And by “lame UX” I mean “it does not work well if you decide to push slightly different code than the requested changes – so the requester has to go inspect the code again and withdraw their requested changes if they approve of it”. It’s not well done.
It would have REALLY HELPED A TON if somebody told me, just once, the following:
“Unless a PR commenter tells you ‘this is blocking, please don’t merge without reviewing’ or something similar, please feel free to outright ignore any other remarks because they are 90% likely to be minor”.
Just. Say. It. Once.
They never did.
I am still sad because I liked the company and the people – before resentment set in anyway. It remained one of the very few places where I would actually return to. But, the CTO and his right hand guy are not ever responding to my messages – I was sending them one every 4 months or so, several times, and finally took the hint and stopped.
Don’t say that to me, say it to about 3/4 of all programmers I ever worked with (likely hundreds at this point). We were talking stuff like “do not use guards” without any elaboration to which I replied “they give us a small extra safety net, the code is already written and no further time will be spent on this, why should I remove it now? we will lose the time I spent on that so far and will also lose out on safety”, and things just gradually spiraled down after other 4-5 of these.
You and I agree, many others don’t, sadly.
We did that but those never got anywhere so I was lost as to why we kept making them. Felt like this was a social hack to make the commenter shut up above anything else. Since it’s not nice to be told “let it go”, they just told you “if you have objections, make a new ticket and new PR(s)”.
All in all, we as programmers should know better than to assume anything. IMO during onboarding somebody should tell you, in no uncertain terms, what exactly is the policy regarding PR/MR reviews and comments.
Don’t assume anything, ever. What’s super obvious to you might be mind-blowing to me, and vice versa.
I realized that "body_match" and "case_match" were not fair since they weren’t invoking a function. After wrapping them in a closure, they have much more similar results to the other two. I posted the updated code below the results.
So this was the average-looking result when running all four:
Name ips average deviation median 99th %
body_match 69.51 14.39 ms ±17.42% 14.73 ms 21.57 ms
dot 69.42 14.41 ms ±16.79% 14.89 ms 21.16 ms
case 69.26 14.44 ms ±17.05% 14.94 ms 21.15 ms
fn_match 68.94 14.51 ms ±17.22% 14.89 ms 21.36 ms
Comparison:
body_match 69.51
dot 69.42 - 1.00x slower +0.0189 ms
case 69.26 - 1.00x slower +0.0516 ms
fn_match 68.94 - 1.01x slower +0.120 ms
The first three jump around a lot with fn_match coming in last pretty regularly (though not by much). I’m realizing that after compilation, dot and case are the exact same code so not really anything interesting there.
And this was the average-looking result for just "fn_match" and "dot".
Name ips average deviation median 99th %
dot 70.21 14.24 ms ±16.65% 14.72 ms 21.20 ms
fn_match 69.58 14.37 ms ±17.12% 14.87 ms 21.62 ms
Comparison:
dot 70.21
fn_match 69.58 - 1.01x slower +0.131 ms
Here’s the updated bench code:
Mix.install([
{:benchee, "~> 1.0", only: :dev}
])
dot = fn %{bar: bar} ->
bar
end
match = fn foo ->
foo.bar
end
body_match = fn foo ->
%{bar: bar} = foo
bar
end
case_case = fn foo ->
case foo do
%{bar: bar} -> bar
end
end
inputs = %{"map" => %{bar: "bar"}}
Benchee.run(
%{
"dot" => fn map ->
for _i <- 1..1_000_000 do
dot.(map)
end
end,
"fn_match" => fn map ->
for _i <- 1..1_000_000 do
match.(map)
end
end,
"body_match" => fn map ->
for _i <- 1..1_000_000 do
body_match.(map)
end
end,
"case" => fn map ->
for _i <- 1..1_000_000 do
case_case.(map)
end
end
},
inputs: inputs
)
EDIT: conclusion seems to be that they are all basically the same, so I wouldn’t consider performance a decider of what to use.
I wonder what percentage of threads you three derail into rants about previous workplaces . Maybe we need a thread that no-one else can see where y’all can go on and on about it til the cows come home.