[Tutor] Good Taste Question: Using SQLite3 in Python

[Jugurtha Hadjar]

On 04/29/2015 09:50 AM, Peter Otten wrote:
>
> My random observations:
>
> (1) find() and create() look very similar. Try hard to factor out common
> code. You might even combine it into a single method ensure_balance(phone).
>

I'll think about it. I can't see for now how I would combine the two 
(each are triggered by different sets of conditions), but there's 
probably a better way.

> (2) According to
>
> https://docs.python.org/dev/library/sqlite3.html#using-the-connection-as-a-context-manager
>
> - the context manager automatically commits
> - you can run sql directly on the connection
>

I read that but I can't recall for the life of me why I didn't use it 
that way. Maybe I didn't understand it well or something broke, etc. 
I'll look into it.

> It might by a good idea to refactor init_db() to just return the connection
> and then use it as
>
>      with self.init_db() as conn:
>          return conn.execute(
>              self.QUERIES['FIND_PHONE'],
>              (self.phone,)).fetchone() is not None
>
> If you need a cursor you can easily get one from the connection. If you want
> more complex handling later you can always turn init_db() into a
> contextmanager (see
> <https://docs.python.org/dev/library/contextlib.html#contextlib.contextmanager>
> ).
>

Yes, that's the way it was but I didn't like conn.cursor and I didn't 
know how to use context managers (still don't). So I wanted to have 
something working first, and then improving it now.


> (3) Catching Exception is overly broad. You will also catch a typo like
>
> cursor.execute(
>      self.QUERIES['CERATE'],
>      (self.phone, seed_balance)
> )
>
> where the proper fix is to modify the script. Only catch exceptions that you
> can actually handle. Example: a table doesn't exist, and you want to create
> it lazily on first access. Let all other exceptions just bubble up. It may
> seem lazy, but a complete traceback is invaluable for identifying and fixing
> a bug.
>

Right. This was also pointed by Alan so it must be really shabby. I'll 
look into it.

> (4) self.ERROR.format(e.args[0])
>
> is probably a string with len() > 0, and thus True in a boolean context.
>  From that follows an exception in the find() method in
>
>>                  self.known = self.find()
>
> causes the following if-suite to be run
>
>>                  if self.known:
>>                          self.balance = self.get_balance()
>
> and if get_balance() has a same idea of proper exception handling
>
> self.balance will end up containing an error message.

Nice, really nice :)

>
> (5) From point (4) I conclude that you don't have unit tests that cover your
> code. You should really write some of these before pondering about stylistic
> issues. Unit tests
>
> - help avoid errors
> - make changes in the code less of a gamble because if the tests succeed
> after the change you can be confident the change didn't introduce an error
> - what you might not expect: how drastically tests affect the style of your
> code. You'll see yourself thinking "How can I test that?" with every line of
> code that you write, and that alone will improve the quality of your code.
>

I have simple scripts that test for specific things, but it's not really 
professional neither does it follow any structure, etc.

I'll improve the code following the recommendations on this thread..

Thanks, Peter.

-- 
~Jugurtha Hadjar,