Help cleaning up some code

odeits odeits at gmail.com
Sun Mar 8 08:07:55 CET 2009


On Mar 7, 10:58 pm, odeits <ode... at gmail.com> wrote:
> On Mar 7, 1:07 pm, Scott David Daniels <Scott.Dani... at Acm.Org> wrote:
>
>
>
> > odeits wrote:
> > > I am looking to clean up this code... any help is much appreciated.
> > > Note: It works just fine, I just think it could be done cleaner.
>
> > > The result is a stack of dictionaries. the query returns up to
> > > STACK_SIZE ads for a user. The check which i think is very ugly is
> > > putting another contraint saying that all of the ni have to be the
> > > same.
>
> > Well, the obvious way to get your constraint is by changing your SQL,
> > but if you are going to do it by fetching rows, try:
>
> >      FIELDS = 'ni adid rundateid rundate city state status'.split()
> >      ni = UNSET = object() # use None unless None might be the value
> >      stack = []
> >      rows = self.con.execute(adquerystring, (user,STACK_SIZE)).fetchall()
> >      for row in rows:
> >          ad = dict()
> >          for field in FIELDS:
> >              ad[field] = row[field]
> >          for field in 'city', 'state':
> >              if ad[field] is None:
> >                  ad[field] = 'None'
> >          if ni != ad['ni']:
> >              if ni is UNSET:
> >                  ni = ad['ni']
> >              else:
> >                  break
> >          stack.append(ad)
>
> > --Scott David Daniels
> > Scott.Dani... at Acm.Org
>
> Taking from several suggestions this is what i have come up with for
> now:
>
>          for row in  ifilter(lambda r: r['ni'] == rows[0]['ni'],rows):
>             ad = dict()
>
>             keys = row.keys() # if python 2.6
>             keys =
> ['ni','adid','rundateid','rundate','city','state','status'] # if
> python 2.5
>
>             for index in row.keys():
>                 if row[index] is None:
>                     ad[index] = 'None'
>                 else:
>                     ad[index] = row[index]
>             stack.append(ad)
>             print row
>
> the test to see if the ad is valid is placed in the ifilter so that I
> dont build the dictionary unnecessarily. and the None special case is
> fairly simple to read now. The None case would even be irrelevant if i
> could get the damn xmlrpc to allow null. sigh. anyhow. thanks for all
> of your input, it is definitely better than it was ;)

For those of you who asked about the SQL the full function is here.
The connection is to a sqlite database with the row_factory set to
sqlite.Row


 def get(self,user):
        '''
            Fetches an ad for USER, assigns the ad and returns a
dictionary that represents an ad
        '''

        stack = []
        aduserquerystring = "SELECT ni, adid, rundateid, rundate,
city, state, status, priority,time FROM ads NATURAL JOIN rundates
NATURAL JOIN newspapers WHERE  ( status in (1,3) and user = ?) "
        expiredadquerystring  = "SELECT ni, adid, rundateid, rundate,
city, state, status, priority,time FROM ads NATURAL JOIN rundates
NATURAL JOIN newspapers WHERE   ( status = 1 AND time < DATETIME
('NOW', '-%d MINUTES') ) LIMIT 0,?"%(MINUTES_TO_EXPIRE,)
        adquerystring = "SELECT ni, adid, rundateid, rundate, city,
state, status , priority, time FROM ads NATURAL JOIN rundates NATURAL
JOIN newspapers WHERE (status IN (0,2) AND priority IN ( SELECT
priority FROM users NATURAL JOIN groups WHERE user = ? ))  ORDER BY
status DESC, priority ASC, ni ASC,  time ASC, adid ASC LIMIT 0,?"

        rows = self.con.execute(aduserquerystring,(user,)).fetchall()
        if len(rows) ==  0:
            rows = self.con.execute(expiredadquerystring,
(STACK_SIZE,)).fetchall()
        if len(rows) ==  0:
            rows = self.con.execute(adquerystring,
(user,STACK_SIZE)).fetchall()
        print user
        keys =
['ni','adid','rundateid','rundate','status','city','state']

        for row in  ifilter(lambda r: r['ni'] == rows[0]['ni'], rows):
            ad = dict( )

            for key in keys:
                if row[key] is None:
                    ad[key] = 'None'
                else:
                    ad[key] = row[key]


            stack.append(ad)
            print row

        self.con.executemany('UPDATE ads SET user = ?, status = CASE
(status) WHEN 1 THEN 1 WHEN 0 THEN 1 WHEN 2 THEN 3 END WHERE adid = ?',
[(user, ad['adid']) for ad in stack])
        self.con.commit()


        return stack






More information about the Python-list mailing list