undefined | Better HN

0 pointsryandrake1y ago0 comments

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!

0 comments

12 comments · 1 top-level

buggy62571y ago· 11 in thread

> sometimes reducing it to a one line lambda.

Please don't do this :) Readable code is better than clever code!

n4r91y 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);

davidgay1y 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).

1 more reply

throwaway8899001y 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.

banannaise1y ago

> "lambda x : x if x else 1"

I think what you're looking for is "x or 1"

gopher_space1y ago

My cleverest one-liners will block me when I come back to them unless I write a few paragraphs of explanation as well.

1 more reply

johnnyanmac1y 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).

bluefirebrand1y 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_1y 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.

1 more reply

vkou1y 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?

1 more reply

jcelerier1y 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; }

johnnyanmac1y 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.

j / k navigate · click thread line to collapse

0 comments

12 comments · 1 top-level

buggy62571y ago· 11 in thread

> sometimes reducing it to a one line lambda.

Please don't do this :) Readable code is better than clever code!

n4r91y 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);

davidgay1y 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).

1 more reply

throwaway8899001y 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.

banannaise1y ago

> "lambda x : x if x else 1"

I think what you're looking for is "x or 1"

gopher_space1y ago

My cleverest one-liners will block me when I come back to them unless I write a few paragraphs of explanation as well.

1 more reply

johnnyanmac1y 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).

bluefirebrand1y 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_1y 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.

1 more reply

vkou1y 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?

1 more reply

jcelerier1y 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; }

johnnyanmac1y ago

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.

j / k navigate · click thread line to collapse