121–130 of 265 posts
Earlier quoted context omitted.
Unit tests catch that kind of stuff
The code works perfectly - there is no issue that a unit test could catch... unless you are spying on internally created objects to a method and verifying that certain functions are called some number of times for given data.
Earlier quoted context omitted.
You're not verifying the observable behavior of your application? lmao
How would you suggest tests around: void func() { printEvens(someCall().stream().filter(n -> n % 2 == 0).toList()); } void printEvens(List nums) { nums.stream().filter(n -> n % 2 == 0).forEach(n -> System.out.println(n)); } The first filter is redundant in this example. Duplicate code checkers are checking for exactly matching lines. I am unaware of any linter or static analyzer that would flag this. What's more, uni…
You could write a test that makes sure the output of someCall is passed directly to printeven without being modified.
The example as you wrote is hard to test in general. It's probably not something you would write if your serious about testing.
My experience with using AI tools for code review is that they do find critical bugs (from my retrospective analysis, maybe 80% of the time), but the signal to noise ratio is poor. It's really hard to get it not to tell you 20 highly speculative reasons why the code is problematic along with the one critical error. And in almost all cases, sufficient human attention would also have identified the critical bug - so hu…
My experience with using AI tools for code review is that they do find critical bugs (from my retrospective analysis, maybe 80% of the time), but the signal to noise ratio is poor. It's really hard to get it not to tell you 20 highly speculative reasons why the code is problematic along with the one critical error. And in almost all cases, sufficient human attention would also have identified the critical bug - so hu…
None of these tools perform particularly well and all lack context to actually provide a meaningful review beyond what a linter would find, IMO. The SOTA isn't capable of using a code diff as a jumping off point. Also the system prompts for some of them are kinda funny in a hopelessly naive aspirational way. We should all aspire to live and breathe the code review system prompt on a daily basis.
Opus 4.5 catches all sorts of things a linter would not, and with little manual prompting at that. Missing DB indexes, forgotten migration scenarios, inconsistencies with similar services, an overlooked edge case. Now I'm getting a robot to review the branch at regular intervals and poking holes in my thinking. The trick is not to use an LLM as a confirmation machine. It doesn't replace a human reviewer. I don't see…
I would on the whole prefer a 'lint-style' tool to catch most stuff because they don't hallucinate.
But obviously they don't catch everything so an LLM-based review seems like an additional useful tool.
Earlier quoted context omitted.
Opus 4.5 catches all sorts of things a linter would not, and with little manual prompting at that. Missing DB indexes, forgotten migration scenarios, inconsistencies with similar services, an overlooked edge case. Now I'm getting a robot to review the branch at regular intervals and poking holes in my thinking. The trick is not to use an LLM as a confirmation machine. It doesn't replace a human reviewer. I don't see…
I came to the same conclusion and ended up wiring a custom pipeline with LangGraph and Celery. The markup on the SaaS options is hard to justify given the raw API costs. The main benefit of rolling it yourself seems to be the control over context retrieval—I can force it to look at specific Postgres schemas or related service definitions that a generic CI integration usually misses.
Not guaranteed though of course.
Earlier quoted context omitted.
How would you suggest tests around: void func() { printEvens(someCall().stream().filter(n -> n % 2 == 0).toList()); } void printEvens(List nums) { nums.stream().filter(n -> n % 2 == 0).forEach(n -> System.out.println(n)); } The first filter is redundant in this example. Duplicate code checkers are checking for exactly matching lines. I am unaware of any linter or static analyzer that would flag this. What's more, uni…
Idk how exactly to do it in cpp becasue I'm not familiar with the tooling You could write a test that makes sure the output of someCall is passed directly to printeven without being modified. The example as you wrote is hard to test in general. It's probably not something you would write if your serious about testing.
#include
#include
#include
std::vector someCall()
{
return {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
}
void printEvens(const std::vector& nums)
{
std::ranges::for_each(nums, [](int n)
{
if (n % 2 == 0)
{
std::cout data = someCall();
std::vector tmp;
std::ranges::copy_if(data,
std::back_inserter(tmp),
[](int n) { return n % 2 == 0; }
);
printEvens(tmp);
return 0;
}
Nothing in there is wrong. There is no test that would fail short of going through the hassle of creating a new type that does some sort of introspection of its call stack to verify which function its being called in.
Likewise, identify if a linter or other static analysis tool could catch this issue.
Yes, this is a contrived example and it likely isn't idiomatic C++ (C++ isn't my 'native' language). The actual code in Java was more complex and had a lot more going on in other parts of the files. However, it should serve to show that there isn't a test for printEvens or someCall that would fail because it was filtered twice. Additionally, it should show that a linter or other static analysis wouldn't catch the problem (I would be rather impressed with one that did).
From ChatGPT a code review of the code: https://chatgpt.com/share/69780ce6-03e0-8011-a488-e9f3f8173f...
LLMs writing code, and then LLMs reviewing the code. And when customers run into a problem with the buggy slop you just churned out, they can talk to a LLM chat bot. Isn't it just swell?
Earlier quoted context omitted.
> but the signal to noise ratio is poor Nail on the head. Every time I've seen it applied, its awful at this. However this is the one thing I loathe in human reviews as well, where people are leaving twenty comments about naming and then the actual FUNCTIONAL issue is just inside all of that mess. A good code reviewer knows how to just drop all the things that irk them and hyperfocus on what matters, if there's a fun…
Yeah or worse like my boss. We don't have a style guide. But he always wants style changes in every PR, and those style changes are some times contradictory across different PRs. Eventually I've told him "if your comment does not affect performance or business logic, I'm ignoring it". He finally got the message. The fact that he accepted this tells me that deep down he knew his comments were just bike shedding.
Then, going through their code, they make excuses about their code not meeting the same standards they demand.
As the other responder recommends, a style guide is ideal, you can even create an unofficial one and point to it when conflicting style requests are made
Earlier quoted context omitted.
You just completely contradicted yourself then.
Not sure how? Meant this: > The agentic coding tools and review tools I want my team (and myself) to have access to are ones that ones that force an explicit knowledge interview & acquisition process during authoring and involve the engineer more intricately in the whole flow.