4 ms·

I messed around with Copilot for a while and this is one of the things that actually really impressed me. It was very good at taking a messy block of code, and

I messed around with Copilot for a while and this is one of the things that actually really impressed me. It was very good at taking a messy block of code, and simplifying it by removing unnecessary stuff, sometimes reducing it to a one line lambda. Very helpful!

buggy6257 2y ago
> sometimes reducing it to a one line lambda. Please don't do this :) Readable code is better than clever code!
- bluefirebrand 2y ago
  Especially "clever" code that is AI generated! At least with human-written clever code you can trust that somebody understood it at one point but the idea of trusting AI generated code that is "clever" makes my skin crawl
  - Terr_ 2y ago
    Also, the ways in which a (sane) human will screw-up tend to follow internal logic that other humans have learned to predict, recognize, or understand.
    - ben_w 2y ago
      Most devs I've worked with are sane, unfortunately the rare exceptions were not easy to predict or understand.
  - vkou 2y ago
    Who are all these all these engineers who just take whatever garbage they are suggested, and who, without understanding it, submit it in a CL? And was the code they were writing before they had an LLM any better?
    - arkh 2y ago
      > Who are all these all these engineers who just take whatever garbage they are suggested, and who, without understanding it, submit it in a CL? My guess would be engineers who are "forced" to use AI, already mailed management it would be an error and are interviewing for their next company. Malicious compliance: vibe code those new features and let maintainability and security be a problem for next employees / consultants.
- throwaway889900 2y ago
  Sometimes a lambda is more readable. "lambda x : x if x else 1" is pretty understandable and doesn't need to be it's own separately defined function. I should also note that development style also depends on tools, so if your IDE makes inline functions more readable in it's display, it's fine to use concisely defined lambdas. Readablity is a personal preference thing at some point after all.
  - gopher_space 2y ago
    My cleverest one-liners will block me when I come back to them unless I write a few paragraphs of explanation as well.
    - ethbr1 2y ago
      >> Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -- Brian Kernighan https://github.com/dwmkerr/hacker-laws#kernighans-law https://github.com/dwmkerr/hacker-laws#kernighans-law
  - johnnyanmac 2y ago
    Ymmv. Know your language and how it treats such functions on the low level. It's probably fine for Javascript, it might be a disaster in C++ (indirectly).
  - deleted 2y ago
    [deleted]
  - banannaise 2y ago
    > "lambda x : x if x else 1" I think what you're looking for is "x or 1"
- jcelerier 2y ago
  Who says that the one line lambda is less clear that a convoluted 10-line mess doing dumb stuff like if(fooIsTrue) { map["blah"] = bool(fooIsTrue); } else if (!fooIsTrue) { map["blah"] = false; }
  - johnnyanmac 2y ago
    My experience in unmanaged legacy code bases. If it's an actual one liner than sure. Use your ternaries and closures. But there is some gnarly stuff done in some attempt to minimize lines of code. Most of us aren't in some competitive coding organization. And I know it's intentional, but yes. Add some mindfulness to your implementation Map["blah"] = fooIsTrue; I do see your example in the wild sometimes. I've probably done it myself as well and never caught it.
- n4r9 2y ago
  Are you telling me you've never seen code like this: var ageLookup = new Dictionary<AgeRange, List<Member>>(); foreach (var member in members) { var ageRange = member.AgeRange; if (ageLookup.ContainsKey(ageRange)) { ageLookup[ageRange].Add(member); } else { ageLookup[ageRange] = new List<Member>(); ageLookup[ageRange].Add(member); } } which could instead be: var ageLookup = members.ToLookup(m => m.AgeRange, m => m);
  - davidgay 2y ago
    I'm of the opinion that var ageLookup = new Dictionary<AgeRange, List<Member>>(); foreach (var member in members) { ageLookup.getOrCreate(member.AgeRange, List::new).add(member); } is more readable in the long-term... (less predefined methods/concepts to learn).
    - n4r9 2y ago
      Where is `getOrCreate` defined? Is it a custom extension method? There's also a chance we're thinking in different languages. I was writing C#, yours looks a bit more like C++ maybe? Readability incorporates familiarity but also conciseness. I suppose it depends what else is going on in the codebase. I have a database access class in one of my solutions where `ToLookup` is used 15 times; yes you have to learn the concept, but it's an inbuilt method and it's a massive benefit once you grok it.

buggy6257 2y ago

> sometimes reducing it to a one line lambda. Please don't do this :) Readable code is better than clever code!

bluefirebrand 2y ago
Especially "clever" code that is AI generated! At least with human-written clever code you can trust that somebody understood it at one point but the idea of trusting AI generated code that is "clever" makes my skin crawl
- Terr_ 2y ago
  Also, the ways in which a (sane) human will screw-up tend to follow internal logic that other humans have learned to predict, recognize, or understand.
  - ben_w 2y ago
    Most devs I've worked with are sane, unfortunately the rare exceptions were not easy to predict or understand.
- vkou 2y ago
  Who are all these all these engineers who just take whatever garbage they are suggested, and who, without understanding it, submit it in a CL? And was the code they were writing before they had an LLM any better?
  - arkh 2y ago
    > Who are all these all these engineers who just take whatever garbage they are suggested, and who, without understanding it, submit it in a CL? My guess would be engineers who are "forced" to use AI, already mailed management it would be an error and are interviewing for their next company. Malicious compliance: vibe code those new features and let maintainability and security be a problem for next employees / consultants.
throwaway889900 2y ago
Sometimes a lambda is more readable. "lambda x : x if x else 1" is pretty understandable and doesn't need to be it's own separately defined function. I should also note that development style also depends on tools, so if your IDE makes inline functions more readable in it's display, it's fine to use concisely defined lambdas. Readablity is a personal preference thing at some point after all.
- gopher_space 2y ago
  My cleverest one-liners will block me when I come back to them unless I write a few paragraphs of explanation as well.
  - ethbr1 2y ago
    >> Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -- Brian Kernighan https://github.com/dwmkerr/hacker-laws#kernighans-law https://github.com/dwmkerr/hacker-laws#kernighans-law
- johnnyanmac 2y ago
  Ymmv. Know your language and how it treats such functions on the low level. It's probably fine for Javascript, it might be a disaster in C++ (indirectly).
- deleted 2y ago
  [deleted]
- banannaise 2y ago
  > "lambda x : x if x else 1" I think what you're looking for is "x or 1"
jcelerier 2y ago
Who says that the one line lambda is less clear that a convoluted 10-line mess doing dumb stuff like if(fooIsTrue) { map["blah"] = bool(fooIsTrue); } else if (!fooIsTrue) { map["blah"] = false; }
- johnnyanmac 2y ago
  My experience in unmanaged legacy code bases. If it's an actual one liner than sure. Use your ternaries and closures. But there is some gnarly stuff done in some attempt to minimize lines of code. Most of us aren't in some competitive coding organization. And I know it's intentional, but yes. Add some mindfulness to your implementation Map["blah"] = fooIsTrue; I do see your example in the wild sometimes. I've probably done it myself as well and never caught it.
n4r9 2y ago
Are you telling me you've never seen code like this: var ageLookup = new Dictionary<AgeRange, List<Member>>(); foreach (var member in members) { var ageRange = member.AgeRange; if (ageLookup.ContainsKey(ageRange)) { ageLookup[ageRange].Add(member); } else { ageLookup[ageRange] = new List<Member>(); ageLookup[ageRange].Add(member); } } which could instead be: var ageLookup = members.ToLookup(m => m.AgeRange, m => m);
- davidgay 2y ago
  I'm of the opinion that var ageLookup = new Dictionary<AgeRange, List<Member>>(); foreach (var member in members) { ageLookup.getOrCreate(member.AgeRange, List::new).add(member); } is more readable in the long-term... (less predefined methods/concepts to learn).
  - n4r9 2y ago
    Where is `getOrCreate` defined? Is it a custom extension method? There's also a chance we're thinking in different languages. I was writing C#, yours looks a bit more like C++ maybe? Readability incorporates familiarity but also conciseness. I suppose it depends what else is going on in the codebase. I have a database access class in one of my solutions where `ToLookup` is used 15 times; yes you have to learn the concept, but it's an inbuilt method and it's a massive benefit once you grok it.