Ancestors

Written by Jon Lasser is Hanging in there on 2025-01-24 at 02:36

I think I found a bad code smell today, but I wonder if there's a good reason I can't fathom, because the smells were stacked three deep.

Smell the first: instead of just writing mydict[mykey] = myvalue, every new key written to the dict was via a function update_dict, like so:

mydict = update_dict(mydict, mykey, myvalue)

1/3

=> More informations about this toot | More toots from disappearinjon@wandering.shop

Written by Jon Lasser is Hanging in there on 2025-01-24 at 02:36

Smell the second: that update function actually called mydict.update internally, with a dict inline containing just the relevant key and value, so something like:

local_dict = mydict.update({key: value})

return local_dict

2/3

=> More informations about this toot | More toots from disappearinjon@wandering.shop

Toot

Written by Jon Lasser is Hanging in there on 2025-01-24 at 02:40

But the real joy was smell the third: the update_dict function named their first parameter 'dict'.

That's right, they (locally, in this function) stepped on the reserved word 'dict' in Python.

It's been deployed a year and a half. To its credit, it does technically work. But pee-yew!

The first of these I could maybe see, if you had a validation routine crammed in there. But the second and the third? Who would write code like that, and why?

Am I missing something?

3/3

=> More informations about this toot | More toots from disappearinjon@wandering.shop

Descendants

Written by Rachel Stantz on 2025-01-24 at 02:58

@disappearinjon sounds like a function that wasn’t that well written to begin with got refactored into something useless over time

=> More informations about this toot | More toots from arjache@toot.cat

Written by Jon Lasser is Hanging in there on 2025-01-24 at 02:59

@arjache that’s the least bad explanation I’ve heard so far…

=> More informations about this toot | More toots from disappearinjon@wandering.shop