please comment - my first classes

[Anton Muhin]

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.