On 02Jul2017 11:02, Andrew Z <form...@gmail.com> wrote:
I'd appreciate your suggestions for a better approach to the following task.
I have 2 files ( 2 classes). One (ClassA) has all logic related to the main
workflow of the program. Another (DB), I like to offload all operations with a
DB ( sql3 in this case).
I'm trying to pass the connection to the main class, but having problems. One
of them, is i can't pass the conn as a parameter to the function in one
(ClassA.abc()), because i inherit it ( function abc() ).
I created a self.DBConnection field, but i'm not sure if i'm on the right
path...
Code is gutted to highlight the problem.
Unfortunately you have gutted the "writeTicks" method, making it harder to see
your intent.
You separation is ok, but normally one would try to entire conceal the
unerlying db connection (the sqlite3 connection) from the man class. So you
wouldn't "pass the connection to the main class".
Normally I would have your DB class represent an open (or openable, if you
wanted to defer that) database connection. So your main class would go:
def __init__(self, ...other args...):
self.db = DB(location="blah.sqlite")
def abc(self, reqId: int):
self.db.writeTicks(reqId)
I wouldn't be passing in "self" (your ClassA instance) or self.DBconnection at
all. You'd only pass "self" if the "DB" instance needed more information from
ClassA; normally you'd just pass that information to writeTicks() along with
reqId, so that the DB needs no special knowledge about ClassA.
I've also got a bunch of fine grained remarks about your code that you can take
or leave as you see fit:
one.py:
from .DB import *
Try to avoid importing "*". It sucks all the names from "DB" into your own
namespace. Arguably you only need the DB class itself - all the other
functionality comes with it as methods on the class. So:
from DB import DB
class ClassA(OtherObject):
def __init__(self):
self.DBConnection = sql3.Connection
It isn't obvious why you need this. In my example above I'd just make a DB
instance and save it as self.db; unless you're controlling different backends
that would be all you need.
def abc(self, reqId: int):
DB.writeTicks(self,self.DBConnection,reqId))
Here you're calling the writeTicks method on the DB class itself. I wouldn't be
making that a class method; I'd make it an instance method on a DB instance,
so:
self.db.writeTicks(reqId)
unless there's more to writeTicks (which you've left out).
DB.py:
Try not to name modules that same as their classes - it leads to confusion. I'd
call it "db.py" and make the earlier import:
from db import DB
import sqlite3 as sql3
This feels like an pointless abbreviation.
[...]
class DB(object):
db_location = ''
# db_location = '../DB/pairs.db'
db_location appears to be a class default. These are normally treats as one
would a "constant" in other languages. Stylisticly, this normally means you'd
write the name in upper case, eg:
DEFAULT_DB_LOCATION = '../DB/pairs.db'
def __init__(self, location='../DB/pairs.db'):
db_location = location
And using that would normally look like this:
def __init__(self, location=None):
if location is None:
location = self.DEFAULT_DB_LOCATION
print(current_fn_name(),' self.db_location =
{}'.format(db_location))
try:
with open(db_location) as file:
pass
except IOError as e:
print("Unable to locate the Db @
{}".format(db_location))
I'd just os.path.exists(db_location) myself, or outright make the db connection
immediately.
Also, and this actually is important, error messages should got the the
program's standard error output (or to some logging system). So your print
would look like:
print("Unable to locate the Db @ {}".format(db_location), file=sys.stderr)
Also, normal Python practie here would not be to issue an error message, but to
raise an exception. That way the caller gets to see the problem, and also the
caller cannot accidentally start other work in the false belief that the DB
instance has been made successfully. So better would be:
raise ValueError("Unable to locate the Db @ {}".format(db_location))
def reqConnection(self):
try:
con = sql3.connect(self.db_location)
con.text_factory = str
except sql3.Error as e:
print("Error %s:".format( e.args[0]))
sys.exit(1)
It is generally bad for a class method (or, usually, any funtion) to abort the
program. Raise an exception; that way (a) the caller gets to see the actual
cause of the problem and (b) the caller can decide to abort or try to recover
and (c) if the caller does nothing the program will abort on its own, doing
this for free.
Effectively you have embedded "polciy" inside your reqConnection method,
generally unwelcome - it removes the ability for the caller to implement their
own policy. And that is an architectural thing (where the policy lies).
return con
The easy way to raise the exception here is just to not try/except at all,
thus:
def reqConnection(self):
return sql3.connect(self.db_location)
or if you really need that text_factory:
def reqConnection(self):
con = sql3.connect(self.db_location)
con.text_factory = str
return con
def write(self, con : sql3.Connection, tickId: int):
con.execute( blah)
However I'd make the connection a singleton attribute of the DB class. So I'd
usually have __init__ make the connection immediately (which saves you having
to "probe" the location:
def __init__(self, ...):
...
self.con = sql3.connect(self.db_location)
and then write() would go:
def write(self, tickId: int):
self.con.execute(blah)
and as you can see that _removes_ any need to pass the connection back to the
caller - you don't need to expose an reqConnection method at all, or manage it
in the caller. Instead, ClassA can just store the DB instance itself, and let
DB look after all the specifics. That is exactly the kind of thing class
encapsulation is meant to achieve: the caller (Class A) can wash its hands of
the mechanisms, which are not its problem.
Cheers,
Cameron Simpson <c...@zip.com.au>
--
https://mail.python.org/mailman/listinfo/python-list
