[Tutor] Proper SQLite cursor handling?

[Cameron Simpson]

On 04Jul2021 20:33, boB Stepp <robertvstepp at gmail.com> wrote:
>3)  Am I handling the database stuff properly so far?

Just to this, there's some stuff to criticise here.

>        (
>            self.game_id,
>            _,  # Already have game_name value.
>            self.strategy,
>            self.num_reserve_cards,
>            self.num_foundation_cards,
>            self.reserve_card_value,
>            self.foundation_card_value,
>            self.total_score,
>        ) = self._get_game_data()

This is quite fragile. If the columns in your game table change, even in 
their order, this will assign the wrong things to the various 
attributes. You can have sqlite3 return a Row object instead of a bare 
tuple of values for each row. There's an example in the module docs in 
the "Row Objects" section. Adapting it, after you connect to the 
database, add:

    self.con.row_factory = sqlite3.Row

Then the row fetch methods return Row insteance, which still act like 
tuples but also have attributes named after the columns. So the 
assignment above would become:

    game_row = self._get_game_data()
    self.game_id = game_row.game_id
    self.strategy = game_row.strategy
    self.num_reserve_cards = game_row.num_reserve_cards
    self.num_foundation_cards = game_row.num_foundation_cards
    self.reserve_card_value = game_row.reserve_card_value
    self.foundation_card_value = game_row.foundation_card_value
    self.total_score = game_row.total_score

>    def _get_game_data(self) -> Tuple[int, str, str, int, int, int, 
>    int, int]:

This would return an sqlite3.Row now.

>        """Retrieve game data from database."""
>        cur = self.con.cursor()
>        cur.execute(
>            f"""
>        SELECT * FROM games WHERE game_name = {self.game_name};
>        """
>        )

Normally you try not to SELECT * because it returns all the column 
values, which makes things hairy if you add columns - instead you'd 
select exactly what you want and nothing more. For example, you're not 
using the game_name.

However, with a Row object being returned you can pick what you want out 
of the row as above. Still, it is better practice to SELECT just the 
columns you want. If nothing else, this reduces what gets returned. In 
larger programmes and databases that can be important.

Also, very importantly, this is the wrong way to insert values into an 
SQL query.  The format field {self.game_name} will just get the bare 
game name inserted raw into the SQL, which will likely be a syntax error 
or be misread as a column name. See this document:

    https://xkcd.com/327/

The execute method accepts a dict for things to fill in in the SQL.  
There's an example in the docs:

    cur.execute("select * from people where name_last=:who and age=:age", {"who": who, "age": age})

That will place syntacticly correct values at the ":who" and ":age" 
placeholders in the SQL string, using the values from the dict. Do that 
instead.

>        game_data = cur.fetchall()[0]

You ccould just fetchone().

>        if not game_data:

This test is too late. game_data won't be empty or false, you'll get an 
exception during the fetch. You could go:

    game_rows = cur.fetchall()
    if not game_rows:
        # new game
    else:
        # we expect exactly one row, so the assignment below expects 
        # exactly one element in game_rows
        game_data, = game_rows

Cheers,
Cameron Simpson <cs at cskk.id.au>