On 04/29/2015 09:06 AM, Alan Gauld wrote:
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.


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)


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


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

Roger that.

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


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

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.


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


as above



Thank you for the feedback, Alan.


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

Reply via email to