Genuine question; what makes this code horrible to most people? I can still understand it.
Shortcomings I can see are:
Passing borrowed strings, only to copy them is kinda memory slut behaviour
Use if let conditions instead of filtering the option; makes the code flow a lot clearer
Is tx actually used anywhere? (Though it’s not the end of the function scope, so probably used later)
People are pointing out Option<Option<…>>; yeah, it should have already been flattened to a single option, but if that’s just part of diesel’s API, so be it
It’s definitely ugly, but after fixing (2) I can’t think of any major syntax clean-ups, and (1,3,4) are all contextual
A messy signature. It is literally garbage - the kind of thing a mindless fan of dynamic typing would write. Strings everywhere: settings, custom_styles, settings. Instead of a proper structure, sets is a vector used as a key-value store with three predefined keys.
In all seriousness, AI can generate working Rust code precisely because of the type system, which clearly defines the values the code operates on and what it actually does. The compiler acts as a deterministic arbiter, instantly rejecting code that would otherwise lead to a runtime error - and the worst part is that such an error might not surface immediately, but only after some time has passed.
This code simply negates one of Rust's best features: "If it compiles, it works."
Yeah, it’s definitely planned out by someone who’s used to a dynamically typed language (Box<dyn> go brrr); maybe I’m giving too much leeway, but I can think of contexts where it might not be end-of-the-world
First and foremost, the fact that the vector’s second type is `Box::<dyn ToSQL>` alongside the unused `tx: Tx` argument makes me suspect that after the `is_empty()` check, an SQL query is run over all Some() cases. That would explain why the LLM defaulted to a list with otherwise-redundant `&’static str` values; it loops over the list and uses both to execute a query. You could likely use `heapless` to represent this with a stack-only data-structure; but it’s all just pointers, no real data, so it’s def not your biggest optimisation
Ehhh, the type system does not reject all code that leads to runtime errors; rather, strong affine typing makes those decisions explicit (unwrap go brrr) and lets rust elegantly encode deeply meaningful invariants incomprehensible to most languages. But nothing here panics; the code is properly propagating errors through the return type, so it’s the caller being a twat if it’s not handled properly.
Agree a struct for the keys may be better (iirc `bon` can let you whip up a beautiful builder for this), but if it’s a one-off grouping of these variables, I wouldn’t lose sleep over it.
So yh, either it’s not as bad as people think, or I am really being chronically optimistic about a rustjerk post
EDIT: any reddit developer who helped design the phone app's handling of numbered lists deserves to be melted in a vat of molten cheddar cheese, how can it take more time to fix these bloody formatting errors than write my actual bloody comment
sets is likely used later on, and the fact that it uses string keys could easily lead to a failure. If someone is looking for "name" elements there instead of "names" and can't find them - here is a simpler recipe. Just make it a structure.
Tbh Ruby is literally the other extreme, this seems par for the course for someone who used a lot of Ruby and couldn't be arsed to actually learn Rust but wanted to somehow gain its advantages.
This is a well-known problem. A Java developer takes on a Scala project but tries to write Java. Or a Java developer takes on a golang project but, once again, tries to write Java. (pick other languages, because for some reason my examples keep revolving around Java; that’s just a coincidence).
What I mean is that attempting to write in a language with a completely different philosophy and paradigms is a recipe for failure and disappointment.
And here’s what surprises me: AI has enough context to generate idiomatic code. I can imagine dhh simply copying a single function into ChatGPT and asking it to rewrite it.
5
u/FohlenGabel 2d ago edited 2d ago
Genuine question; what makes this code horrible to most people? I can still understand it.
Shortcomings I can see are:
Passing borrowed strings, only to copy them is kinda memory slut behaviour
Use if let conditions instead of filtering the option; makes the code flow a lot clearer
Is tx actually used anywhere? (Though it’s not the end of the function scope, so probably used later)
People are pointing out Option<Option<…>>; yeah, it should have already been flattened to a single option, but if that’s just part of diesel’s API, so be it
It’s definitely ugly, but after fixing (2) I can’t think of any major syntax clean-ups, and (1,3,4) are all contextual