[Tutor] Good Taste Question: Using SQLite3 in Python

[Jugurtha Hadjar]

Hello, all..

I have a class with methods that access a database (SQLite3). I have
included an excerpt showin reading and writing and would like to know if
I'm doing it right. (i.e: Is it bad code and what to improve).

Here are some improvements and my rationale (check my thinking):

- Initially, each method had its SQL statement(s) inside, but I grouped
all statements in a dictionary, with operations as keys, as a class
'constant' as per previous advice on this mailing list.

- Each method used sqlite3 module on its own, but it was repetitive so I
put that part in its own method `init_db` that returns a tuple
consisting in a connection and a cursor.

- Sometimes there was an exception raised, so I used `try` in `init_db`.

- Methods closed the connection themselves, so I used `with` in
`init_db` instead of `try`, as it would close the connection
automatically and rollback (I hope I'm not making this up).

Here's the excerpt (`DB_FILES` and `QUERIES` are not included here for
more clarity).

Thank you.



def __init__(self, phone):

# Get preliminary information on user and make them
# available.

self.phone = phone
self.known = self.find()

if self.known:
self.balance = self.get_balance()
else:
self.balance = None

def init_db(self):
with sqlite3.connect(self.DB_FILE) as conn:
return conn, conn.cursor()

def find(self):
'''Find the phone in the users database.'''

(__, cursor) = self.init_db()
try:
cursor.execute(
self.QUERIES['FIND_PHONE'],
(self.phone,)
)
found = cursor.fetchone()
return True if found else False
except Exception as e:
return self.ERROR.format(e.args[0])

def create(self, seed_balance):
''' Create a database entry for the sender.'''

conn, cursor = self.init_db()
try:
cursor.execute(
self.QUERIES['CREATE'],
(self.phone, seed_balance)
)
conn.commit()
except Exception as e:
return self.ERROR.format(e.args[0])

[Alan Gauld]

Post by Jugurtha Hadjar
- Initially, each method had its SQL statement(s) inside, but I grouped
all statements in a dictionary, with operations as keys, as a class
'constant' as per previous advice on this mailing list.

We can't see that in the code you posted.
In principle its an acceptable strategy. A close alternative
would be to name the queries as explicit class variables.
So for example you would use:

cur.execute(self.find_phone_query,(....))

Its less typing and less error prone in that you get a
syntax error on mistyping rather than a run time exception.

Post by Jugurtha Hadjar
- Each method used sqlite3 module on its own, but it was repetitive so I
put that part in its own method `init_db` that returns a tuple
consisting in a connection and a cursor.

That's a common approach.

Post by Jugurtha Hadjar
- Sometimes there was an exception raised, so I used `try` in `init_db`.
- Methods closed the connection themselves, so I used `with` in
`init_db` instead of `try`, as it would close the connection
automatically and rollback (I hope I'm not making this up).

Try/except and with do different things but in this case
you can get away with it. The only loss is that your errors
here are not formatted in the same way as the other messages.

Post by Jugurtha Hadjar
Here's the excerpt
# Get preliminary information on user and make them
# available.

Consider using docstrings rather than comments to describe the method
I see you do that below...

Post by Jugurtha Hadjar
self.phone = phone
self.known = self.find()
self.balance = self.get_balance()
self.balance = None
return conn, conn.cursor()
'''Find the phone in the users database.'''
(__, cursor) = self.init_db()

Don't use the __ 'variable' here, be explicit, it makes
maintenance much easier.

Post by Jugurtha Hadjar
cursor.execute(
self.QUERIES['FIND_PHONE'],
(self.phone,)
)
found = cursor.fetchone()
return True if found else False
return self.ERROR.format(e.args[0])

Don't catch Exception, it's too wide. Catch the
actual errors that might arise, be as specific as possible.

Post by Jugurtha Hadjar
''' Create a database entry for the sender.'''
conn, cursor = self.init_db()
cursor.execute(
self.QUERIES['CREATE'],
(self.phone, seed_balance)
)
conn.commit()
return self.ERROR.format(e.args[0])

as above
--
Alan G
Author of the Learn to Program web site
http://www.alan-g.me.uk/
http://www.amazon.com/author/alan_gauld
Follow my photo-blog on Flickr at:
http://www.flickr.com/photos/alangauldphotos


_______________________________________________
Tutor maillist - ***@python.org
To unsubscribe or change subscription options:
https://mail.python.org/mailman/listinfo/tutor

[Jugurtha Hadjar]

Post by Alan Gauld
In principle its an acceptable strategy. A close alternative
would be to name the queries as explicit class variables.
cur.execute(self.find_phone_query,(....))
Its less typing and less error prone in that you get a
syntax error on mistyping rather than a run time exception.

That's what I had initially but I used dictionaries for regexes I match
the messages against to determine the type, I thought I'd organize stuff
of the same nature together in a dictionary.

JUNK = 'JUNK'

RE_TYPE = {
'TYPE1' : re.compile(r'xxx'),
'TYPE2' : re.compile(r'yyy'),
...
'TYPEn' : re.compile(r'aaa'),
}

Then to get the type of the message, I just have to do:

gen = (k for k in self.RE_TYPE if RE_TYPE[k].findall(msg_text))
msg_type = next(gen, self.JUNK)

Post by Alan Gauld

Post by Jugurtha Hadjar
- Methods closed the connection themselves, so I used `with` in
`init_db` instead of `try`, as it would close the connection
automatically and rollback (I hope I'm not making this up).

Try/except and with do different things but in this case
you can get away with it. The only loss is that your errors
here are not formatted in the same way as the other messages.

I wrote it to do one specific job that's to be followed by some other
task that _would_ raise an exception.

Maybe it's lazy and am relying on the methods that call `init_db` to
deal with the error. `init_db` just returns a connection and a cursor..
If the file didn't exist, it'd be created but without schema, the
calling method (like `create` or `find` _will_, however produce an error
as it tries to read/write.

Post by Alan Gauld
Consider using docstrings rather than comments to describe the method
I see you do that below...

Roger that.

Post by Alan Gauld

Post by Jugurtha Hadjar
(__, cursor) = self.init_db()

Don't use the __ 'variable' here, be explicit, it makes
maintenance much easier.

I actually searched specifically for something like this. In MATLAB,
there's the ~ that does this. I'm not trying to write Python in a MATLAB
style, but I was glad I found it here:

http://docs.python-guide.org/en/latest/writing/style/#create-an-ignored-variable

I was only interested in the cursor in that method. `init_db` returned
only a connection in the beginning, but I didn't want to write
conn.cursor every time.

Post by Alan Gauld

Post by Jugurtha Hadjar
cursor.execute(
self.QUERIES['FIND_PHONE'],
(self.phone,)
)
found = cursor.fetchone()
return True if found else False
return self.ERROR.format(e.args[0])

Don't catch Exception, it's too wide. Catch the
actual errors that might arise, be as specific as possible.

Thanks for bringing that to my attention.

Post by Alan Gauld

Post by Jugurtha Hadjar
''' Create a database entry for the sender.'''
conn, cursor = self.init_db()
cursor.execute(
self.QUERIES['CREATE'],
(self.phone, seed_balance)
)
conn.commit()
return self.ERROR.format(e.args[0])

as above

Thank you for the feedback, Alan.

[Alan Gauld]

Post by Jugurtha Hadjar

Post by Alan Gauld

Post by Jugurtha Hadjar
(__, cursor) = self.init_db()

Don't use the __ 'variable' here, be explicit, it makes
maintenance much easier.

I actually searched specifically for something like this. In MATLAB,
there's the ~ that does this. I'm not trying to write Python in a MATLAB
http://docs.python-guide.org/en/latest/writing/style/#create-an-ignored-variable

I've seen this before and strongly disagree with it.
I suspect the author has never had to run a major
maintenance project!

A single underscore is, I agree, far worse that a dunder symbol
but both are a maintenance nightmare. They are ambiguous,
difficult to remember, easy to overlook and if you change
your mind and decide you need to use it later it's a whole
new name to go back and introduce (or worse be tempted to use the
meaningless symbol). And if you need another 'throw-away' name
later you use the same one, then forget there are two uses
and try to use it anyway (even in debugging) and get
completely wrong values, that may or may not look different
to what you expect. (looking different is good - you can
detect it easily, looking similar is very, very, bad!)...
its just a horror story waiting to trip you up.

It's far better to have a short meaningful name that is never
used than a bland, meaningless, generic symbol.
--
Alan G
Author of the Learn to Program web site
http://www.alan-g.me.uk/
http://www.amazon.com/author/alan_gauld
Follow my photo-blog on Flickr at:
http://www.flickr.com/photos/alangauldphotos


_______________________________________________
Tutor maillist - ***@python.org
To unsubscribe or change subscription options:
https://mail.python.org/mailman/listinfo/tutor

[Roel Schroeven]

Post by Alan Gauld

Post by Jugurtha Hadjar
http://docs.python-guide.org/en/latest/writing/style/#create-an-ignored-variable

I've seen this before and strongly disagree with it.

I disagree with your disagreement. I'll try to explain.

Post by Alan Gauld
They are ambiguous, difficult to remember, easy to overlook

I'm not sure what exactly you mean by that.

Post by Alan Gauld
and if you change your mind and decide you need to use it later
it's a whole new name to go back and introduce

True, but I don't see how that is a problem. At that point the variable
is only used at one point, so you only have to rename it at that one place.

Post by Alan Gauld
(or worse be tempted to use the meaningless symbol).

That would be bad indeed, but I think a very minimal amount of
discipline is enough to avoid that.

Post by Alan Gauld
And if you need another 'throw-away' name later you use the same one, then forget
here are two uses and try to use it anyway (even in debugging) and get
completely wrong values, that may or may not look different
to what you expect.

The whole point of ignored variables is that you don't use them. If you
use them, they're not exactly ignored variables. It doesn't matter if
you use __ once or twice or many times more; all of them are to be ignored.

Post by Alan Gauld
(looking different is good - you can
detect it easily, looking similar is very, very, bad!)...
its just a horror story waiting to trip you up.

I'm not sure in what way __ can lead to horror stories. Do you have an
example to fuel my imagination?

Post by Alan Gauld
It's far better to have a short meaningful name that is never
used than a bland, meaningless, generic symbol.

By 'short meaningful name', do you mean something like 'dummy' or
'ignorethis' as in

basename, dummy, ext = filename.rpartition('.')

or rather something like

basename, separator, ext = filename.rpartition('.')

In the first case, I prefer __ over dummy exactly because to me it's
clearer at a glance that the name is one-use only and the value is to be
ignored.
In the second case, using a 'real' name like 'separator' means I now
have to mentally keep track of it since as far as I can see at that
point it might be used later in the code.


To me, using _ or __ decreases the cognitive load because they tell me I
don't have to remember anything about them, since they're not going to
be used later. Read and forget.



Best regards,
Roel
--
The saddest aspect of life right now is that science gathers knowledge
faster than society gathers wisdom.
-- Isaac Asimov

Roel Schroeven

_______________________________________________
Tutor maillist - ***@python.org
To unsubscribe or change subscription options:
https://mail.python.org/mailman/listinfo/tutor

[Peter Otten]

Post by Roel Schroeven

Post by Alan Gauld

Post by Jugurtha Hadjar
http://docs.python-guide.org/en/latest/writing/style/#create-an-ignored-variable

I've seen this before and strongly disagree with it.

I disagree with your disagreement. I'll try to explain.

Post by Alan Gauld
They are ambiguous, difficult to remember, easy to overlook

I'm not sure what exactly you mean by that.

Post by Alan Gauld
and if you change your mind and decide you need to use it later
it's a whole new name to go back and introduce

True, but I don't see how that is a problem. At that point the variable
is only used at one point, so you only have to rename it at that one place.

Post by Alan Gauld
(or worse be tempted to use the meaningless symbol).

That would be bad indeed, but I think a very minimal amount of
discipline is enough to avoid that.

Post by Alan Gauld
And if you need another 'throw-away' name later you use the same one, then forget
here are two uses and try to use it anyway (even in debugging) and get
completely wrong values, that may or may not look different
to what you expect.

The whole point of ignored variables is that you don't use them. If you
use them, they're not exactly ignored variables. It doesn't matter if
you use __ once or twice or many times more; all of them are to be ignored.

Post by Alan Gauld
(looking different is good - you can
detect it easily, looking similar is very, very, bad!)...
its just a horror story waiting to trip you up.

I'm not sure in what way __ can lead to horror stories. Do you have an
example to fuel my imagination?

Post by Alan Gauld
It's far better to have a short meaningful name that is never
used than a bland, meaningless, generic symbol.

By 'short meaningful name', do you mean something like 'dummy' or
'ignorethis' as in
basename, dummy, ext = filename.rpartition('.')
or rather something like
basename, separator, ext = filename.rpartition('.')
In the first case, I prefer __ over dummy exactly because to me it's
clearer at a glance that the name is one-use only and the value is to be
ignored.
In the second case, using a 'real' name like 'separator' means I now
have to mentally keep track of it since as far as I can see at that
point it might be used later in the code.
To me, using _ or __ decreases the cognitive load because they tell me I
don't have to remember anything about them, since they're not going to
be used later. Read and forget.

Inside a function there's a middle ground, a leading underscore:

def whatever():
...
basename, _extsep, ext = filename.rpartition(os.extsep)
...

The name makes it clear what _extsep is supposed to contain, and the
underscore indicates that you are not using the name.

You can later question the decision that you are not interested in _extsep
-- in the example you may decide that you want to discriminate between

"somefile." and "somefile"

Even if you stick with the original design I find it more readable to state
explicitly what you are throwing away.

My whatever() function is an example where the name of the to-be-ignored
variable led to a change in the code: I started with _separator and then
remembered that the separator need not be a dot.

Admittedly I don't expect to use an OS where os.extsep != "." anytime soon,
but you get the idea...

_______________________________________________
Tutor maillist - ***@python.org
To unsubscribe or change subscription options:
https://mail.python.org/mailman/listinfo/tutor

[Alan Gauld]

Post by Roel Schroeven

Post by Alan Gauld

Post by Jugurtha Hadjar
http://docs.python-guide.org/en/latest/writing/style/#create-an-ignored-variable

I've seen this before and strongly disagree with it.

I disagree with your disagreement. I'll try to explain.

Post by Alan Gauld
They are ambiguous, difficult to remember, easy to overlook

I'm not sure what exactly you mean by that.

If something exists in a function, over time somebody will use it.
Especially during debugging, priont is a dangerous tool in these cases.
Simple fact of life. If it exists it should be visible (not too
short or incongruous) Trying to visually scan for _ or even
__ is hard. Also different fonts make _ and __ hard to
distinguish.

Post by Roel Schroeven

Post by Alan Gauld
and if you change your mind and decide you need to use it later
it's a whole new name to go back and introduce

True, but I don't see how that is a problem. At that point the variable
is only used at one point, so you only have to rename it at that one place.

It is if its only used once - see below.
Renaming the wrong variable (especially ising search./replace
in the editor) is a very common way to introduce new bugs into
working code! Having spent nearly 10 years running 3 different
maintenance teams I know how easy it is for new programmers(*)
to start changing things they don;t understand, especially
when under time pressure.

(*)And maintenance teams are almost always comprised of a
combination of new or very old (read jaded) programmers.
The top players are reserved for the exciting new stuff!

Post by Roel Schroeven

Post by Alan Gauld
(or worse be tempted to use the meaningless symbol).

That would be bad indeed, but I think a very minimal amount of
discipline is enough to avoid that.

Its almost impossible, A maintenance team is usually working
on a ratio of 10-50,000 lines of code per programmer per project
and typically 3-5 projects. So if you have to hold 50-150,000
lines of code in mind its very easy to slip something in as
a 'quick fix' intending to fix it next time then forget about
it. Especially if you have to check in two bug fixes before
you get to go home tonight....

Maintenance programmers are rarely the people who wrote the
code, and they rarely have much personal pride invested in
the code they have to fix. Even if they are the original
developer they will have moved on to new projects and
fixing 5 year old code is a low priority. Its all about
human nature.

Post by Roel Schroeven
The whole point of ignored variables is that you don't use them.

That's the point when they are created, but too often a
use is found for these things. The original author is never in a
position to say 'this will never be used in the future'
that's just too idealistic. And the original author is
very unlikely to be the long tem maintainer. Even in
open-source where people tend to stick with projects
for a few years its not true. In commercial code the
author probably leaves the company within 6 months of
writing the code. He may never even see it go into
production...

Post by Roel Schroeven
use them, they're not exactly ignored variables. It doesn't matter if
you use __ once or twice or many times more; all of them are to be ignored.

Absolutely. nothing can be guaranteed to be ignored over
time, to start out with tat asumption is tom lay the seeds
of chaos later.

Post by Roel Schroeven

Post by Alan Gauld
(looking different is good - you can
detect it easily, looking similar is very, very, bad!)...
its just a horror story waiting to trip you up.

I'm not sure in what way __ can lead to horror stories. Do you have an
example to fuel my imagination?

None in Python - I've never run a Python maintenance squad,
but I have led assembler, C/C++, SQL, Perl and COBOL: teams.
In every case there have been attempts to use variables that
had been intended to be unused - in some cases randomly
initialized/unassigned, reassigned etc. I once saw a C
function with the same dummy variable use 4 times - although
that did have a name, but it was a placeholder like x.

But in Perl I've seen the infamous $_ abused many times
(although that's slightly different since it magically takes
on certain values, implicit rather than explicit) Python tries
to avoid these things but the whole _/__ variable scheme is
IMHO one of its worst features.

Post by Roel Schroeven

Post by Alan Gauld
It's far better to have a short meaningful name that is never
used than a bland, meaningless, generic symbol.

By 'short meaningful name', do you mean something like 'dummy' or
'ignorethis' as in
basename, dummy, ext = filename.rpartition('.')
or rather something like
basename, separator, ext = filename.rpartition('.')

The latter. or even 'sep' as shorter than 'separator'. At least
it gives some clue to its meaning and can be used later
if needed. Because its not expected to be used typing the
few extra characters once doesn't hurt. But it makes
scaffold code, debug code and extension code much easier
to write.

Post by Roel Schroeven
In the first case, I prefer __ over dummy exactly because to me it's
clearer at a glance that the name is one-use only and the value is to be
ignored.

The intent may be clearer but dummy is much easier to
scan, search for and spot visually.

Post by Roel Schroeven
In the second case, using a 'real' name like 'separator' means I now
have to mentally keep track of it since as far as I can see at that
point it might be used later in the code.

It will get reused anyhow, just accept that and give it a sensible name.
If you don't intend to use it ignore it.

Post by Roel Schroeven
To me, using _ or __ decreases the cognitive load because they tell me I
don't have to remember anything about them, since they're not going to
be used later. Read and forget.

But they will be. Almost for certain. It's human nature and the nature
of code maintenance. If it's there somebody will find a use for it. The
fact that 5 or 10 years earlier the author didn't intend for it to be
used is immaterial.
--
Alan G
Author of the Learn to Program web site
http://www.alan-g.me.uk/
http://www.amazon.com/author/alan_gauld
Follow my photo-blog on Flickr at:
http://www.flickr.com/photos/alangauldphotos


_______________________________________________
Tutor maillist - ***@python.org
To unsubscribe or change subscription options:
https://mail.python.org/mailman/listinfo/tutor

[Roel Schroeven]

...
Trying to visually scan for _ or even __ is hard. Also different
fonts make _ and __ hard to distinguish.
...
But they will be. Almost for certain. It's human nature and the nature
of code maintenance. If it's there somebody will find a use for it. The
fact that 5 or 10 years earlier the author didn't intend for it to be
used is immaterial.

Summarizing a bit, I think you make two main points (please correct me
if I'm wrong):

[1] Visually scanning for _ or __ is hard, and _ are __ hard to
distinguish from each other.

Personally, I find it easy to scan for them, but I think I can see whee
you're coming from. Python tends to prefer words and tends to dislike
symbols compared to e.g. C, C++, and certainly Perl. One could argue
that using _ or __ goes against that, though to me it's not a problem.
We're still very far from Perl's line noise.
It's true that _ and __ can be difficult to be distinguished from each
other, but that's also not a problem to me, since I don't care about
their values.


[2] Inevitably, sooner or later someone somewhere will start using _ or
__ despite the fact that by convention they should not be used.

I have to admit that I have no experience programming in larger teams,
and was blissfully unaware of the problems you describe. I think I can
see how it might be better to avoid __ rather than try to enforce good
coding discipline.


I still feel __ to be valuable, but I can see now where your dislike for
it is coming from. Thank you for your insights!


Best regards,
Roel
--
The saddest aspect of life right now is that science gathers knowledge
faster than society gathers wisdom.
-- Isaac Asimov

Roel Schroeven

_______________________________________________
Tutor maillist - ***@python.org
To unsubscribe or change subscription options:
https://mail.python.org/mailman/listinfo/tutor

[Dave Angel]

Post by Roel Schroeven

...
Trying to visually scan for _ or even __ is hard. Also different
fonts make _ and __ hard to distinguish.
...
But they will be. Almost for certain. It's human nature and the nature
of code maintenance. If it's there somebody will find a use for it.
The fact that 5 or 10 years earlier the author didn't intend for it to
be used is immaterial.

Summarizing a bit, I think you make two main points (please correct me
[1] Visually scanning for _ or __ is hard, and _ are __ hard to
distinguish from each other.
Personally, I find it easy to scan for them, but I think I can see whee
you're coming from. Python tends to prefer words and tends to dislike
symbols compared to e.g. C, C++, and certainly Perl. One could argue
that using _ or __ goes against that, though to me it's not a problem.
We're still very far from Perl's line noise.
It's true that _ and __ can be difficult to be distinguished from each
other, but that's also not a problem to me, since I don't care about
their values.
[2] Inevitably, sooner or later someone somewhere will start using _ or
__ despite the fact that by convention they should not be used.
I have to admit that I have no experience programming in larger teams,
and was blissfully unaware of the problems you describe. I think I can
see how it might be better to avoid __ rather than try to enforce good
coding discipline.
I still feel __ to be valuable, but I can see now where your dislike for
it is coming from. Thank you for your insights!

Well, are you aware that _ has a meaning in the debugger? It holds the
last value of an expression that wasn't assigned to a variable. or
something like that.

So even if you don't have any coworkers, you might trip on someone
else's assumptions.

Post by Roel Schroeven

3*4

12

Post by Roel Schroeven

print(_)

12

Post by Roel Schroeven
Best regards,
Roel

[Roel Schroeven]

Post by Dave Angel
Well, are you aware that _ has a meaning in the debugger? It holds the
last value of an expression that wasn't assigned to a variable. or
something like that.

Yes, I know the meaning of _ in Python's interactive mode. It's
something I sometimes use for convenience, but never really rely on. To
me interactive mode is for trying out simple things, not for serious stuff.

Anyway, as explained in this style guide it's better to use __ anyway
instead of _:

http://docs.python-guide.org/en/latest/writing/style/#create-an-ignored-variable
--
The saddest aspect of life right now is that science gathers knowledge
faster than society gathers wisdom.
-- Isaac Asimov

Roel Schroeven

_______________________________________________
Tutor maillist - ***@python.org
To unsubscribe or change subscription options:
https://mail.python.org/mailman/listinfo/tutor

[Alan Gauld]

Post by Roel Schroeven
Summarizing a bit, I think you make two main points (please correct me

Your quite correct.

I'm probably a bit paranoid but as I said I spent a
significant bit of my production-programming career
(about 40%) on maintenance projects and I've seen the
horrors that come from uninitialised variables, misused
names etc etc. And any kind of meaningless symbiol
(even if the meaning is not needed at the time).
And in a maintenance team where you are targetted
to fix, say, 4 bugs per day, a "horror" is anything
that adds more than say 15-20 minutes to the process!

Post by Roel Schroeven
[1] Visually scanning for _ or __ is hard, and _ are __ hard to
distinguish from each other.
Personally, I find it easy to scan for them, but I think I can see whee
you're coming from. Python tends to prefer words and tends to dislike
symbols

Yes but worse in this cae is tat Python uses __ for special
purposes so if there is a __ near to a variable name the reader
is likely to assume its just a pre-pended dunder marker rather
than a separate name. It also makes searching for dunders harder
since there are likely to be quite a few in the code already.
(yes you can create a regex search but making it reliable
is quite tricky.)

Post by Roel Schroeven
[2] Inevitably, sooner or later someone somewhere will start using _ or
__ despite the fact that by convention they should not be used.
I have to admit that I have no experience programming in larger teams,
and was blissfully unaware of the problems you describe.

Again my experience is not typical of the tutor forum, but
some tutor readers may go on to bigger things and bad habits
once learned become permanent...

My smallest project team was 7 programmers producing 80,000
lines of code. Next smallest 18, producing 250,000.
Most of my projects have had over 100 programmers and several
million LOC (usually C++ or COBOL)

One project had over 1000 programmers and produced 60 million
LOC using 7(?) different programming languages. (I ran one of
the maintenance teams of ~10 coders all fresh from uni')

Like I say I get paranoid about some things :-)
--
Alan G
Author of the Learn to Program web site
http://www.alan-g.me.uk/
http://www.amazon.com/author/alan_gauld
Follow my photo-blog on Flickr at:
http://www.flickr.com/photos/alangauldphotos


_______________________________________________
Tutor maillist - ***@python.org
To unsubscribe or change subscription options:
https://mail.python.org/mailman/listinfo/tutor

[Laura Creighton]

The fact that _ and __ are intended as throw away values is only clear
to people who have read a particular doc about coding styles. If you
haven't read the doc, you don't know what is going on. I name my
throw away variables junk, and if there are lots of them, for instance
when I am reading from a list of tuples, but only care about the fourth
item in each tuple, its 'junk1, junk2, junk3 ...'

So far I have yet to meet somebody who wasn't immediately aware that
I did not care about those values.

Laura

_______________________________________________
Tutor maillist - ***@python.org
To unsubscribe or change subscription options:
https://mail.python.org/mailman/listinfo/tutor

[Peter Otten]

Post by Jugurtha Hadjar
I have a class with methods that access a database (SQLite3). I have
included an excerpt showin reading and writing and would like to know if
I'm doing it right. (i.e: Is it bad code and what to improve).
- Initially, each method had its SQL statement(s) inside, but I grouped
all statements in a dictionary, with operations as keys, as a class
'constant' as per previous advice on this mailing list.
- Each method used sqlite3 module on its own, but it was repetitive so I
put that part in its own method `init_db` that returns a tuple
consisting in a connection and a cursor.
- Sometimes there was an exception raised, so I used `try` in `init_db`.
- Methods closed the connection themselves, so I used `with` in
`init_db` instead of `try`, as it would close the connection
automatically and rollback (I hope I'm not making this up).
Here's the excerpt (`DB_FILES` and `QUERIES` are not included here for
more clarity).
Thank you.
# Get preliminary information on user and make them
# available.
self.phone = phone
self.known = self.find()
self.balance = self.get_balance()

Post by Jugurtha Hadjar
self.balance = None

return conn, conn.cursor()
'''Find the phone in the users database.'''
(__, cursor) = self.init_db()
cursor.execute(
self.QUERIES['FIND_PHONE'],
(self.phone,)
)
found = cursor.fetchone()
return True if found else False
return self.ERROR.format(e.args[0])
''' Create a database entry for the sender.'''
conn, cursor = self.init_db()
cursor.execute(
self.QUERIES['CREATE'],
(self.phone, seed_balan>ce)
)
conn.commit()
return self.ERROR.format(e.args[0])

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).

(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

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>
).

(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.

(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

Post by Jugurtha Hadjar
self.known = self.find()

causes the following if-suite to be run

Post by Jugurtha Hadjar
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.

(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.

A simple example is your self.find(). How would you test that with different
phone numbers? That's easier when you can pass the phone number as an
argument, so you might change the method's signature.



_______________________________________________
Tutor maillist - ***@python.org
To unsubscribe or change subscription options:
https://mail.python.org/mailman/listinfo/tutor

[Jugurtha Hadjar]

Post by Peter Otten
(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.

Post by Peter Otten
(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.

Post by Peter Otten
It might by a good idea to refactor init_db() to just return the connection
and then use it as
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.

Post by Peter Otten
(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.

Post by Peter Otten
(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

Post by Jugurtha Hadjar
self.known = self.find()

causes the following if-suite to be run

Post by Jugurtha Hadjar
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 :)

Post by Peter Otten
(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.