Settings

Theme

Ask HN: Has anyone else found it harder to review code recently?

28 points by jballanc 2 years ago · 45 comments · 2 min read

Reader

The last 12-18 months or so I've started to realize that I'm having a harder time reviewing code, especially if that code is from a stranger (e.g. on an OSS project) or a new colleague.

I find that I will take a first glance at the code, notice the inclusion of some more advanced concept or technique, and instinctually I'll assume the code is the product of a more seasoned developer. Then, quite often, I'll come across something in the code that doesn't make sense. My initial reaction is to assume the author of this code is using some new technique or approach I'm not familiar with. I'll dig and search and turn myself inside out trying to figure out what I'm missing...only to eventually come to the conclusion that I haven't actually missed anything.

No, the reason the code didn't make sense to me is because the code just...doesn't make sense. It was not the product of some seasoned developer, but rather of a junior developer who has simply made a series of run-of-the-mill junior-developer-mistakes. In the past I would have been able to pick these sorts of mistakes out from a mile away. Now I find I have to see past the patina of pseudo-sophisticated LLM-generated code first before I can start the review process.

dlisboa 2 years ago

I’ve found that junior devs can be incredibly creative and produce seemingly high-level code that they don’t fully understand. Results are often bad because they didn’t consider tradeoffs but still, pretty interesting code. Even before LLMs were a thing, so I wouldn’t attribute it to ChatGPT.

  • jballancOP 2 years ago

    I agree, and in fact I would consider working with junior developers a highlight of my day because there is an inherent virtue in the "beginners mind". And you're not wrong that juniors writing things that seem more complicated than they have any right to be producing is not a new thing. It's more that the patterns have skewed. Over a nearly 20 year career, I've developed pattern matching skills that can catch this kind of code...when produced by humans. It seems that I'll have to adjust my priors now. That's all.

rvdginste 2 years ago

When I read about people using AI to write code, it always seems to me that it would be a lot faster and less hassle if they would write the code themselves, without even considering the fact that it would give them more experience and make them better.

I have used some of those tools myself, and for the code that I could use help of an AI tool, I, again and again, receive junk: code that looks plausible but that does not compile, uses apis or libraries that do not exist and so on. In the end, it just made me waste time.

  • MattPalmer1086 2 years ago

    I find it useful when I'm working with some unfamiliar technology. It can get me up and running much faster, and even debug code to some extent.

    I used it yesterday to fix a problem with my makefile. I couldn't see the issue, gave it to chatGPT. It gave me some code that didn't actually work for some reason, but which had the correct solution to my problem in it. I just ported over the solution to mine, job done. This was after spending a couple of fruitless hours reading documentation and blog posts.

  • steveBK123 2 years ago

    we had a guy on our team who was using ChatGPT aggressively in his awful code, to the point of putting his prompts in as comments

    he did not last long...

    this is the kind of developer you have to explain (repeatedly) why things need to run on servers & service accounts rather than his laptop as his personal ID

  • paiute 2 years ago

    I felt the same about ai until the newer chatgpt. It’s really better than i am. It’s never perfect, but I don’t think I’ll ever go without it now.

hypeatei 2 years ago

Does anyone else not understand the hype with AI generated code? I almost never use AI at $DAYJOB; only for "search engine" type tasks where I'm looking for an answer or perspective on something that's hard to search for with google - not code generation.

Feels like more of a fight with the AI and less time thinking about the bigger picture of the changes being made (e.g. systems involved, business considerations, documentation, etc..)

  • steve_adams_86 2 years ago

    I use it like a search engine (or rubber duck) too. I like to test ideas and form hypotheses, but I don't really like to code with an LLM. Occasionally it's helpful to template something but it's rare that I want to iterate on the foundation an LLM created.

    I suppose the main reasons are that 1) the code convention likely doesn't mesh with what I'm working on, 2) the lack of context awareness, 3) common performance concerns and 4) potential IP issues which admittedly seems like a rare issue yet I'd rather not contend with it.

    There is an exception though. When I'm trying to prototype firmware quickly, I like to use it to generate code for components I haven't used before or recently, and I'm content to rely on it until I've proven out an idea. Past that point though I will almost certainly rip out that code and start fresh using the conventions and patterns I prefer.

  • williamcotton 2 years ago

    Last week I oversaw and architected a number of STDIO Python scripts for ingesting PDFs page-by-page into postgres tsvector, searching, and displaying the text and metadata that was entirely written by an LLM.

    I didn’t “fight” with the machine and I focused solely on the bigger picture. I read the output like I was performing a code review.

  • makeitdouble 2 years ago

    > Does anyone else not understand the hype with AI generated code?

    This can feel maddening, but is basically the approach taken for a few decades in many companies.

    Developpers who think long and hard about their code and bring stability to the code base cost money, sometimes a lot. In comparison many businesses can get away with the "cheap labor with meh output but handled by QA and a management layer" approach.

    That was Taylorism in the early days, outsourcing to random location also came from the same philosophy, the dream of "code modules" from the EJB area that non devs would just mix and match, etc.

    That dream will always live, if AI isn't it, something else will come up as a magical wand that somewhat generate output from a wide cheap "meh" labor base that only needs to be checked and managed by a few knowledgeable people.

  • dinosaurdynasty 2 years ago

    I have to write a lot of boilerplate at $DAYJOB (Go is like that). Copilot really helps decrease the amount of typing I have to do.

mensetmanusman 2 years ago

This sounds like a jobs plan!

AI will create the world‘s largest code mess, while also running the entire world, and everything will be slightly broken like in the movie Brazil.

  • hcarvalhoalves 2 years ago

    10 years from now, the last generation of senior devs will be making money fixing the mess being created right now. LLM-generated code is the new COBOL.

langcss 2 years ago

Warm take?

Just reject and ask for an explanation OR pair with the coder to have them explain the code.

If the reviewer can’t understand it without an explanation the rest of the team can’t understand it unless they git blame then ask the coder, assuming they are still at the company!

The “dump 1000 lines of previously unseen and undiscussed code as a code review at a reviewer” method is an antipattern.

Either the reviewer should be heavily involved or break it into smaller, well explained chunks with design documentation to point to.

This saves everyones time in the long run.

  • snapdaddy 2 years ago

    As a senior developer of 30+ years experience, I agree and was going to post the same thing.

    If code is difficult to understand, I consider that to be a bug. And if, for some reason, the section of code actually is difficult to implement, I would at least want a pretty good comment explaining the context around it and the reasons for the approach taken.

    Not only will this help improve the code, but if it is a senior developer, it will help them to understand where they've become blind to a particular complexity. It is great feedback to have someone explain that they don't understand code that seems 'fine' to you.

    • jballancOP 2 years ago

      In principal I agree, and it's not like I'm losing entire days to this sort of thing (well, once I did, but only once), but there's a tricky interplay of experience + lingering imposter syndrome. I do a fair bit of jumping around between languages and frameworks, so when I, say, come back to JS after being away for 6-9 months and I see something unfamiliar, there's usually a 60/40 split that it's a bug vs something the JS community has decided is a new "industry best practice". Before immediately going to the author asking for an explanation, I attempt to do my due diligence and be sure that I'm on the 60% side of that split.

      It's just that lately the split has been more like 60/30/10 between bug, some new thing, and garbage AI spew.

kirth_gersen 2 years ago

AI generated code is "plausible" seeming, but often has subtle bugs in my experience. Bugs and false assumptions if you don't spend a long time telling the AI exactly what you are trying to do. And if you are a novice, you probably don't know exactly what you are trying to do in the first place.

cbanek 2 years ago

I have had this problem a LOT recently, although in the past it has like you've said, been easier to review code.

First, let me blame microservices. I heard your eyes roll, but just wait a minute.

Second, there's all these new frameworks that kind of sit on top of a language just to make it practically a new language. Things like pydantic and sqlalchemy, and other things that parse data, check data, and move data around are their own kind of magic with anything more than a trivial data model. You have to learn how the whole modeling works. Then try to apply it to what is going on. In the end, I'm generally not sure the code is any better, and it's certainly less readable than just a function that parses your data. The frameworks are more complex though, and have all sorts of features. Features you need to learn when someone new brings that framework into your code.

But AI's can easily generate boilerplate code to handle things that even the author doesn't understand. This is not good.

So now you've got a "simple" microservice that uses a bunch of libraries and things on top of a language. Such as for python, it's pydantic and mypy and numpy, just all sorts of things that are very good, but hard to understand what is going on from a code review basis unless you are also familiar with these frameworks.

To go back to blaming microservices, now that everyone wants to have a bunch of little containers running around, even a simple ecosystem can use all sorts of languages, frameworks, build systems, etc. You have to learn all of them for every service.

To contrast this with how it used to be in my opinion, one product had one ecosystem. One product would be more monolithic, using fewer moving pieces. Once you learned the pieces, of which there were fewer, you can easily review code.

While it can be that many codebases are smaller by using more off the shelf libraries, that smallness hides problems. Many times people don't understand the deep ways that the frameworks work, and where the problems may be, especially with performance and scaling.

But this small code doesn't make nearly as much sense as even larger amounts of just language code where everything is in front of you to review.

In the end, you can over-engineer things in a very hidden way.

  • shrimp_emoji 2 years ago

    > First, let me blame microservices. I heard your eyes roll,

    No, that was the sound of me updooting the post.

kkfx 2 years ago

As a sysadmin I found a crescent level of crappiness and monstrosity that's essentially unmanageable, undebuggable, incomprehensible so well... I dot not even try sometimes to review certain yaml/js monsters...

Most young devs have completely NO knowledge of how a system works, no idea about how to deploy and so on, they are "born on someone else APIs", meaning some cloud vendor and they do not even want to know. The results are fragile big monsters so buggy that no one looks at logs, at least not if something do not break.

to11mtm 2 years ago

I've gotten to where I sometimes have questioned whether code has come from an LLM or was actually written by a human on some OSS stuff.

I'm not really -good- at it but the explanation for false positives has typically been 'someone not up to a task' where it gets to the point I call it as an LLM.

cryptica 2 years ago

Yes, it's been happening more and more over the past decade. 10 years ago, code was on average relatively clean and easy to understand. Concepts like 'high cohesion, loose coupling' were drilled into us. Nowadays, developers barely know what these terms mean. Most developers don't seem to take pride in their work anymore. Seemingly, they're trying to maximize their lock-in factor via complexity. Probably most do it subconsciously, but the effect is so strong and it has gotten so much worse over time that it feels intentional. Like people have been conditioned to add as much complexity as possible. Coding has become mostly unpleasant nowadays because of this.

Most experienced developers know that unnecessary complexity is the absolute worst enemy. You literally cannot overstate the harms of unnecessary complexity... It is absolutely and inherently harmful. But to the junior or mid-level developer, complexity is a sign of intelligence; that, along with the ability to churn out thousands of lines of code per day.

On my own projects, I never allow this complexity, but when you're working for a company, they don't like it if you point out that there is a complexity issue. They'll think that maybe you're just not smart enough and are jealous of or trying to demoralize the 'genius junior dev' who is churning out 2k lines per day! Truly Kafkaesque situation.

I honestly didn't know what to do in my last job. I was doing a lot of PR reviews but I just let the 'most productive' junior dev continue adding complexity because that was what the founder wanted me to do. Every time I tried to talk about reducing complexity, I would get brushed off, so I just stopped trying.

It's quite a ridiculous situation actually. Because all the code I write is high quality; highly maintainable, everyone is able to easily add features and make changes to it, but when I work on other people's ugly, over-engineered code, it's a struggle.

So from the outside, it looks like I'm slow when working with other people's code, and it looks like other developers are fast and adaptable since they can easily work with my code... So basically I look like I'm the one who is a low performer.

The winning strategy is clearly to write over-engineered code, then try to socially engineer the situation so that you only end up working on your own code or other people's high quality maintainable code (if there is such a thing at your company because people who produce such code tend to get laid off)... While at the same time, you need to try to ensnare your colleagues to work on your complex code so that they end up looking unproductive relative to you... Because huge amount of code + visible features is how directors decide on promotions and layoffs... It's always about picking low hanging fruits, sprinkling sugar on top and then personally delivering it to the boss on a silver platter; easy and visible.

Much of software engineering nowadays is social engineering; ensuring that you are only assigned to decent quality maintainable and highly visible projects, always hitchhiking on top of the work produced by good developers and dumping your own low-quality output on others to entrap them. Sigh... Then after some time, these big companies end up with ridiculously low productivity expectations... Which is great for social-scheming low performers who are used to this game of racing to the bottom.

Also, people like me who can see what's going on are never promoted to positions where I can have the last say on such things. It feels like the entire tech economy is just a massive bullshit jobs factory at this stage. All about pretending to be highly productive while in fact being counter-productive.

  • BWStearns 2 years ago

    > 10 years ago, code was on average relatively clean and easy to understand

    We have very different memories of 10 years ago. I just remember peak OOP cargo culting with 10 layers of inheritance and searching `Factory` would OOM your IDE.

  • rvdginste 2 years ago

    > Because all the code I write is high quality…

    That is a big claim to make…

    I do follow you on the complexity of code and that good code should avoid unnecessary complexity (‘unnecessary’ being key here).

    • Brian_K_White 2 years ago

      Given the rest was reasonable I was willing to grant that as "aims for high quality".

      Because it wasn't about how good of a programmer they are, it was about their intentions and principles. "I try" vs "I don't try" already makes the significant relative difference regardless what the absolute values are.

paiute 2 years ago

I’ve been coding over 20 years. I’m guilty. But I’ve also learned that for most domains it doesn’t really matter if you understand the code, if it work, it works. Of course you have to watch for security and safety issues. Maybe ask an llm to explain the code under review?

  • syndicatedjelly 2 years ago

    With all due respect to your experience, please don't go around saying that. I don't know why people like to brag about not understanding things. As professional software engineers, this belief makes us sound incompetent, not humble.

    • paiute 2 years ago

      There’s an ability to understand. Most coders really have no clue how their code works. It’s not a brag, nor incompetence. Does your average dev really understand the underlying instruction set? LLM are like having a team of junior devs. I can quickly skim it, test it, ship it. I’ve worked in all sorts of safety related code. I would trust ai code over some of the crap I’ve seen (and probably wrote myself)

  • saulpw 2 years ago

    This is such a dangerous take these days. Something can appear to "work" in the main case but be grossly or subtly broken for common edge cases. Malicious contributors (see recent xz exploit) are skilled at making code that works but also has subtle security issues. You can't just say "it doesn't really matter if you understand the code" in one breath and then "of course you have to watch for security and safety issues" in the next. How can you watch for those issues if you don't understand the code? And suggesting LLMs makes the deep mistake of thinking that an LLM understands anything at all.

    • paiute 2 years ago

      Tell me how an exception works. I’ve seen bugs caused by people not understanding the underlying mechanism. It doesn’t mean one shouldn’t use them.

      • saulpw a year ago

        Specifically you said "it doesn’t really matter if you understand the code"...the literal code under review. Not the underlying mechanism of an abstraction you're using, which is useful, sure, even if you don't understand how it works under the hood. If you're reviewing the code that implements an abstraction, you better damn well understand how it works under the hood.

Rury 2 years ago

Perhaps a case of over-engineering?

ldjkfkdsjnv 2 years ago

Honestly, I rarely write more than 10 lines of code without an LLM. I copy existing code files into LLM, ask it to add feature I want. Iterate a bunch of times on the solution. Manually fix the last bits. Check in the software.

  • lobito14 2 years ago

    That's not coding, it's prompting.

    • MarkusQ 2 years ago

      Yeah. You tell 'em!

      Real programmers write the code themselves. By hand. In a hex editor. All these new-fangled code generating tool chains like compilers and AI coding assistance are a bunch of coder coddling toad tinkle.

      Now get off my lawn.

      :)

chatmasta 2 years ago

On the non-coding side, I’m seeing lots of docs with sections blatantly copy pasted from ChatGPT.

Pro-tip: if your normal writing includes typos and sloppy formatting, then be careful pasting a block of three bullet points where each point is a title-cased bold string, followed by exactly two sentences, with perfect grammar and no typos.

largelady97 2 years ago

Well I faced the same problems earlier today. I had to fix up my LLM generated code before sending it on to my line manager Steve for him to review it. I ran it through ChatGPT a few times to make it seem liked a seasoned developer has written it and to increase its complexity. Its not working out though. My plan now is to use a multi GPT system. I will feed my ChatGPT code into Google Gemini and go around in a circle at least 10 times. That should put Steve off and buy me some more time.

Keyboard Shortcuts

j
Next item
k
Previous item
o / Enter
Open selected item
?
Show this help
Esc
Close modal / clear selection