please comment - my first classes
Hilbert
hilbert at microsoft.com
Sat Mar 15 02:23:46 EST 2003
Thanks for your comments.
I'm using the lastKey, not the len() method because I want to be able
to delete entries. This way I can just delete the desired key.
I want to refer to bookmarks by their key, not by their value.
I'm not using lists for the same reason.
Thanks,
Hilbert
"Anton Muhin" <antonmuhin at sendmail.ru> wrote in message
news:b4t1q0$grt$1 at news.peterlink.ru...
> I quite agree with Sean about naming conventions too.
>
> Some more notes.
>
> > class bookmark:
> > def __init__(self,group,name,url):
> > self.group=group
> > self.name=name
> > self.url=url
> >
> > def Print(self):
> > print self.group,':',self.name,':',self.url
> it might be written as:
> print ':'.join([self.group, self.name. self.url])
>
> if all are strings. This solution is quite understanable (at least for
> me ;) and might be a bit more effecient.
>
> >
> > def AsTuple(self):
> > return( (group,name,url) )
> >
> > class bookmark_group_db:
> > def __init__(self):
> > self.lastKey = -1
> > self.groups = {}
> >
> > def Add(self,name):
> > self.lastKey += 1
> > self.groups[self.lastKey]=name
> Or
> self.groups[len(self.groups)] = name
>
> This seems to eliminate need for lastKey. BTW, why not to use lists?
> self.groups = []
> self.groups.append(name)
> ?
>
> >
> > def Has(self,key):
> > if self.groups.has_key(key):
> > return 1
> > else:
> > return 0
> Maybe just:
> return self.groups.has_key(key)
>
> or (for lists)
> return 0 <= key < len(self.groups)
>
> >
> > def PrintAll(self):
> > for i in self.groups.keys():
> > print i,':',self.groups[i]
> For dict:
> for i, value in self.groups.iteritems():
> print i, ':', value
>
> For lists (in Python2.3, but it quite easy for Python2.2 too):
> for i, value in enumearte(self.groups):
> print...
>
> >
> > class bookmark_db:
> > def __init__(self):
> > self.lastKey = -1
> > self.bookmarks = {}
> >
> > def Add(self,group,name,url):
> > self.lastKey += 1
> > self.bookmarks[self.lastKey]=bookmark(group,name,url)
> See above
>
> >
> > def AddBookmark(self,src):
> > self.lastKey += 1
> > self.bookmarks[self.lastKey]=src
> See above
>
> >
> > def PrintAll(self):
> > for i in self.bookmarks.keys():
> > self.bookmarks[i].Print()
> See above
> BTW, if you need this functionality often, why not abstract it into a
class?
>
> >
> > def Find(self,sub):
> > matches = bookmark_db()
> > for i in self.bookmarks:
> > if self.bookmarks[i].name.find(sub)>-1 or
> > self.bookmarks[i].url.find(sub)>-1:
> > matches.AddBookmark(self.bookmarks[i])
> > return (matches)
> I personally don't like () in returns, but it's a matter of taste
>
> Does this code work? Don't you mean:
> for bookmark in self.bookmarks:
> if bookmark.name.find(sub) ....?
>
> You may introduce function or method for bookmark matching:
>
> def doesBookmarkMatch(bookmark, sub):
> return bookmark.name.find(..) or bookmark.url.find(...)
>
> And use it.
>
> BTW,several more compact forms:
> matches = [bookmark for bookmark in self.bookmarks if
> doesBookmarkMatch(sub)]
>
> or
> matches = filter(doesBookmarMatch, self.bookmarks)
>
> >
> > def Size(self):
> > return (len(self.bookmarks))
> In some context, you might want to override __len__ method to use
> something like len(bookmark_db())
>
> BTW, your bookmark_db seems to be just a list. Maybe you need several
> functions and stick to a simple list.
>
> >
> [snipped]
>
> HTH,
> Anton.
>
More information about the Python-list
mailing list