search

PhilanthropyDataCommons / service

A project for collecting and serving public information associated with grant applications
GNU Affero General Public License v3.0
8 stars 2 forks source link

Rethink use of `load` for db utility functions #860

Open slifty opened 7 months ago

slifty commented 7 months ago

As part of #228 we have been writing db utility functions for running queries.

We use create as a prefix for insert statements, and we use load as a prefix for select statements. @bickelj has pointed out that load is ambiguous since it could either mean reading from disk to memory, or reading from memory to disk.

We should discuss!

bickelj commented 7 months ago

"Load" in the context of ETL means one thing while "load" in the context of an OS means another. When I see "load" I'm like "what does load mean here?"

I think it's clearest to have the method names use the SQL verbs for methods that ultimately result in SQL statements, namely "insert, select, update, delete." Alternatively, CRUD: "create, read, update, delete." Alternatively, HTTP verbs: "put, get, patch, delete", etc.

bickelj commented 7 months ago

But it's OK, I can get over it eventually : )

slifty commented 2 months ago

@bickelj it's been a while but I want you to know I think of this every time I write code that references load

Are you cool with the CRUD terms you outlined?

We already use create and update for the equivalent operations, so read would fit nicely if that still works for you.

If so, I'll open a PR that patches this up soon!

bickelj commented 2 months ago

@slifty Yes, that sounds great, but super low priority. I'm getting used to the way we use the word "load" but for newcomers to the project it might be helpful, depending on how they see it.