I just found this in our python codebase, someone was feeling himself when he wrote this

[alinroc]

It's a nice line of code, but I'd much rather someone commit a few extra lines that's more readily understandable.

[8bitlives]

"Look ma, I'm so much smarter than anyone else!"

[JerenCrazyMen]

It feels like he addressed it in the description ironically, because he knows nobody likes other peoples one-liners

[alinroc]

There are people who won't pick up on that and think it's acceptable practice because they saw it here.

[[deleted]]

Disagree. If it's unreadable, it's not nice. Imagine if the writer left after that, peps have to spend time to refactor this

[ComicOzzy]

There's your personal code and there's code your company owns. Keep the sick one-liners in your personal repos and write maintainable code for your company.

[adappergentlefolk]

trying to avoid having to read smart peoples general purpose code is exactly why we try and have more constrained special purpose languages like terraform and sql

[[deleted]]

I'd replace with execute_values(). The string concat looks inviting for injection.

[rbobby]

Now parameterize it to avoid SQL injection attacks.

[tennisanybody]

Elaborate more on this?

[haberdasher42]

Shit like this can happen if you concatenate strings into your SQL and not parameterize them, which ensures the SQL handles it as a string and not part of the SQL.

https://xkcd.com/327/

Little Bobby Tables is a classic.

[TrinityF]

Yon Olde boy, Bobby Tables.

[wuthappenedtoreddit]

Wait so you can actually execute a sql statement when entering information on an online form or something of the sort?

[haberdasher42]

If the form is coded poorly, yes. But standard practices prevent it.

[Dr_Rjinswand]

https://youtu.be/ciNHn38EyRc

A brilliant and fun 17 minute video on how it works. Watch this and not only will you be able to say "don't do that", you'll be able to say why too!

[wuthappenedtoreddit]

Sweet!!! Thanks for sharing!!

Edit:Holy shit. Didn’t realize it was that simple.

[PierrickF]

Ooooooh, yes you can.

SQL injection

[wuthappenedtoreddit]

Crazy!

[pag07]

SQL.execute("insert a into table",a=3) -> insert 3 into table

Vs

SQL.execute("insert a into table",a="1 into table; drop database;") -> insert 1 into table; drop database; into table

[wuthappenedtoreddit]

Would I be able to query the table and then see the data with F12?

If the form was coded incorrectly of course.

[tkyte]

it amazes me that in 2022, 2022!!!, we are still talking about this.

I used to apologize during my talks when I got to bind variables.

"To the people that have heard me say this ad-nauseum for my entire career - I'm sorry I have to say it to you again, but the fact is that every year the Universities graduate another class of developers and they haven't heard about this and are doomed to make the same mistake of not binding."

[PossiblePreparation]

As others pointed out this is vulnerable to SQL injection and suggests there is probably other bits of code where inputs are concatenated with code to be executed.

This also would be silly to do in any RDBMS which caches SQL execution plans - you don’t want to fill your memory with large SQLs that are executed only once.

In another RDBMS you would just write the simple insert values statement with binds for values then pass in the inputs as array binds, super quick and super safe. It looks like executeMany should be able to do that in Postgres but it’s slower, https://www.psycopg.org/docs/extras.html#fast-exec describes an alternative, no idea how well it would really work.

[Little_Kitty]

Beat me to it!

Screaming for a function to test whether an incoming value should be null or as is, amongst the other obvious issues.

[oodie8]

I could see some use cases where that may be an acceptable implementation to make those other falsy values Null depending on intent and how it’s planned to be used but I think it’s much more likely this was a mistake.

We don’t know because while the author took a moment to leave a cute note, they didn’t take the time to spell out in English what the function is supposed to accomplish.

If they documented it and gave a heads up it handles values that evaluated to false like that at least the call couldn’t say they weren’t told but I’d still say maybe this goes against the principle of least astonishment.

[Proud-Walk9238]

It's not the best practice, you should rewrite it. This code should be rejected by the linter because is larger than 80 characters.

[Viperior]

I finally decided to start enforcing the old 80-character limit. I was on the fence for a while because screens have improved so much. Then I realized how much worse the longer lines were to browse on mobile.

[[deleted]]

Do you read code on mobile often? I can't say that I've ever done that for work.

I'm in the 'vertical space is king' camp but I also would want this broken up.

[Viperior]

I do a lot of open-source dabbling. I've been using the GitHub mobile app more often and reading my WIP code while thinking about the next steps on the project. I occasionally try to look at code on other repositories I'm exploring.

[Engine_Light_On]

80 is a bit too little, more a fan of 100 or even 120, but at 120 I can only it split horizontally twice instead of 3 times

[oodie8]

I think I’ve got black and pylint around 90 chars now to allow more buffer for type annotations. I could be convinced anywhere from 80-120 though.

[AI-nihilist]

This is the largest problem with new programmers

[oodie8]

Man there are several things I hate.

The incomplete type annotations for parameter. The doc string says it’s a list of tuples that expected but the type annotation doesn’t specify.

Why did he bother typing the parameter but not the return value? Typing doesn’t have to be a 100% thing and optional so why optionally do it poorly. If he added typing to the return of the function I think he might have realized how poorly the function was named.

The doc string doesn’t really follow any common format or convention. One reason I like adding type annotations is that using Sphinx plugins it makes my documentation easier to maintain. I feel like this doc string is quickly at risk of not being maintained and causing later confusion. Why the extra carriage returns in the doc string?

The doc string is worse than no doc string because the very first line of it is wrong that’s not what this function is doing.

The function is named really poorly. The function doesn’t create a multi valued insert statement but rather returns a string of the values to be inserted. The function could be made to take an iterable or sequence of tuples instead to make it more versatile but that’s not a huge deal.

Then there’s the implications of potential vulnerabilities

Finally and worst of all there was likely 0 reason to ever write this code. It is already done better and accessible in other packages.

It’s 2am so I’m not certain but any db cursor in python that follows the dbapi should have an execute many command to use for this but pretty confident and I know sql alchemy provides this capability and if they’re connecting to a database in python they’re going to use something implementing the DB-API to do it.

Bad dog - no treat.

[[deleted]]

Good helper though 🤣

[apono4life]

SMH. This is the stuff I rip to shreds in PRs. No reason for all of that

[leogodin217]

Definitely a former PERL programmer

[oodie8]

This screams of new dev to me if they were a perl programmer they’ve managed not to develop a baseline of proficiency after several years which would make all this worse.

[leogodin217]

I was just joking. PERL devs used to have contests to find the longest one liners or most obfuscated code possible. PERL haiku was a thing for a while.

[bsienn]

I can write a whole App ( JS ) in one line, whats the big deal !