Good or bad class design?

[ShadyTyrant]

I am designing a module to perform some actions for a library database application. The methods are not implemented yet but I was wondering what you all think about nested classes in this situation.

Design One

#!/usr/bin/env python
#   File: .py
# Author: [e-mail]
#   Date: 2009-11-25
#  Notes:
#--------------------------------------------

class Library:
    def __init__(self):
        # Just for reference at this point
        id = 'NULL'
        seriesIndex = 1
        title = ''
        author = ''
        genra = ['fiction', 'non-fiction', 'science fiction', 'technical', 'reference']
        publisher = ''

    #--------------------------------------------

    def AllBooks(self):
        pass

    def SearchById(self):
        pass

    def SearchByTitle(self):
        pass

    def SearchByAuthor(self):
        pass

    def SearchByGenra(self):
        pass

    def SearchByPublisher(self):
        pass

    #--------------------------------------------

    def AddBook(self):
        pass

    def RemoveBook(self):
        pass

    def EditSeriesIndex(self):
        pass

    def EditTitle(self):
        pass

    def EditAuthor(self):
        pass

    def EditGenra(self):
        pass

    def EditPublisher(self):
        pass

Design Two

#!/usr/bin/env python
#   File: .py
# Author: [e-mail]
#   Date: 2009-11-25
#  Notes:
#--------------------------------------------

class Library:
    def __init__(self):
        # Just for reference at this point
        id = 'NULL'
        seriesIndex = 1
        title = ''
        author = ''
        genra = ['fiction', 'non-fiction', 'science fiction', 'technical', 'reference']
        publisher = ''

    #--------------------------------------------
    class Search:
        def AllBooks(self):
            pass

        def ById(self):
            pass

        def ByTitle(self):
            pass

        def ByAuthor(self):
            pass

        def ByGenra(self):
            pass

        def ByPublisher(self):
            pass

    #--------------------------------------------
    class Edit:
        def Add(self):
            pass

        def Remove(self):
            pass

        def SeriesIndex(self):
            pass

        def Title(self):
            pass

        def Author(self):
            pass

        def Genra(self):
            pass

        def Publisher(self):
            pass

[Gribouillis]

I prefer design 1. We don't know anything about the algorithms that you have in mind, so there is no obvious benefit for introducing the classes Search and Edit. With design 1, the Library object appears as a facade behind which you can implement arbitrary complex and flexible algorithms.
Also I don't think it's a good idea to use nested classes in general. There is no gain in nesting classes and they will be more accessible if you implement them linearly at module level.

[ShadyTyrant]

Thank you very much for your input. Let me elaborate on what I plan on doing with this module. I will perform SQL query's on a SQLite database holding information about books. I will be able to add, remove, and edit information about books in the database. The reason I even thought about nested classes is because I see the search and edit methods as two separate features. Though Gribouillis made an excellent point and was exactly right when he said "the Library object appears as a facade behind which you can implement arbitrary complex and flexible algorithms". Design one does seem to be more practical. Anymore thoughts?

[lrh9]

I'm just wondering why the Library class has a container for book information. Is the book information a temporary container?

[ShadyTyrant]

Yes the book information is temporary, and for design purpose only. In other languages like java I would have to define the variable and data type before use. This allows me to think about what variables I will be using and the types of values they will hold. In python I am not required to do this but it helps me to think about the information in my problem.

[programmersbook]

i think think make more sense:

#!/usr/bin/env python
#   File: .py
# Author: [e-mail]
#   Date: 2009-11-25
#  Notes:
#--------------------------------------------

class LibraryManager():
    def filter(self, *args, **kwargs):
        """ Search by kwargs or args"""
        pass
    
    def add(self, *args, **kwargs):
        """ Add by kwargs or args"""
        pass
    
    def remove(self, *args, **kwargs):
        """ Remove by kwargs or args"""
        pass
    
class Library:
    self.manager = LibraryManager()
    
    def __init__(self):
        # Just for reference at this point
        id = 'NULL'
        seriesIndex = 1
        title = ''
        author = ''
        genra = ['fiction', 'non-fiction', 'science fiction', 'technical', 'reference']
        publisher = ''

[woooee]

I would suggest two classes, one that handles the physical details of the SQL database and one for everything else. So, if you want to modify a record, the second class would generate a GUI to ask for info on the record. The info would be passed to the SQL class' lookup function, which would pass the record back. You use a separate class for the SQL functions to have the option of using more than one program to access the database. They would all use the same SQL class so would all have tested routines and would not yield an error in this program but not in that program.

[vegaseat]

I prefer design 1. We don't know anything about the algorithms that you have in mind, so there is no obvious benefit for introducing the classes Search and Edit. With design 1, the Library object appears as a facade behind which you can implement arbitrary complex and flexible algorithms.
Also I don't think it's a good idea to use nested classes in general. There is no gain in nesting classes and they will be more accessible if you implement them linearly at module level.

I tend to agree with Gribouillis. As your program grows you can split out some classes in a linear fashion. Later on, as the bugs are worked out, turn them into modules

[ShadyTyrant]

Thank you all very much for your help in the decision. I have gone with design one and have implemented the features that I want so far. I am going to create a two front ends to implement the module. One command line and the other GUI. You can view the current module at coder profile and I will be posting the finished applications with downloads very soon to coder profile.

[Gribouillis]

Thank you all very much for your help in the decision. I have gone with design one and have implemented the features that I want so far. I am going to create a two front ends to implement the module. One command line and the other GUI. You can view the current module at coder profile and I will be posting the finished applications with downloads very soon to coder profile.

Nice. Just one criticism, I don't like your

except:
    return False

because you're just hiding errors, so that if something goes wrong, your program will never know. You should think about another way to handle the error (if you don't want to write error recovery code, you could send the error to a log file).

[ShadyTyrant]

Actuality I don't think I am hiding errors. I am allowing the error handling to differ depending on the type of front end implementation. Example:

def AddBookTest():
    done = lib.AddBook(title, author, lib.GENRA[1], publisher, seriesIndex)
    if done == True:
        print "Added %s to library" % title
    else:
        print "could not add book to library"

If I was using a GUI took kit I wouldn't use print statements to let the user know of an error I might use dialog boxes of something else.

Here is the unit testing app I was using to implement the module when I was designing it. Please note this app was just a test and is not being used in anyway for my other implementations.

#!/usr/bin/env python
#   File: {filename}
# Author: [e-mail]
#   Date: 2009-11-09
#  Notes:
#--------------------------------------------

import LibrarySql

def AddBookTest():
    done = lib.AddBook(title, author, lib.GENRA[1], publisher, seriesIndex)
    if done == True:
        print "Added %s to library" % title
    else:
        print "could not add book to library"

def SearchAllTest():
    allData = lib.SearchAll()
    if allData == False:
        print 'No Books in library'

def RemoveBookTest():
    done = lib.RemoveBook(6)
    if done == True:
        print "removed from library"
    else:
        print "could not remove book to library"

def SearchByIdTest():
    allData = lib.SearchById(1)
    print allData

def SearchByTitleTest():
    allData = lib.SearchByTitle("HarryP")
    print allData

def SearchByAuthorTest():
    allData = lib.SearchByAuthor("JKR")
    print allData

def SearchByGenraTest():
    allData = lib.SearchByGenra(lib.GENRA[1])
    print allData

def SearchByPublisherTest():
    allData = lib.SearchByPublisher("APUB")
    print allData

def EditTitleTest():
    done = lib.EditTitle(1, "Jack The Ripper")
    if done == True:
        print "Edited title"
    else:
        print "could not edit title"

def EditAuthorTest():
    done = lib.EditAuthor(1, "Jack")
    if done == True:
        print "Edited author"
    else:
        print "could not edit author"

def EditGenraTest():
    done = lib.EditGenra(1, lib.GENRA[3])
    if done == True:
        print "Edited genra"
    else:
        print "could not edit genra"

def EditPublisherTest():
    done = lib.EditPublisher(1, "ANewPUB")
    if done == True:
        print "Edited pub"
    else:
        print "could not edit pub"

def EditSeriesIndexTest():
    done = lib.EditSeriesIndex(1, 4)
    if done == True:
        print "Edited index"
    else:
        print "could not edit index"

seriesIndex = 1
title = 'HarryP'
author = 'JKR'
#genra = ['fiction', 'non-fiction', 'science fiction', 'technical', 'reference']
#genra2 = 'fic'
publisher = 'APUB'

dataPath = "/home/tyrant/Geany/Python/Projects/LibraryDb/librarydb"
lib = LibrarySql.Library(dataPath)

#AddBookTest()
SearchAllTest()
#RemoveBookTest()
#SearchByIdTest()
#SearchByTitleTest()
#SearchByAuthorTest()
#SearchByGenraTest()
#SearchByPublisherTest()

#EditTitleTest()
#EditAuthorTest()
#EditGenraTest()
#EditPublisherTest()
#EditSeriesIndexTest()

[Gribouillis]

I see. Note that handling errors by the means of a return status is not very pythonic. It's a C like way of thinking. A more pythonic approach would be to define a

class LibraryError(Exception):
    pass

and in your methods, you could raise a LibraryError instead of returning False. Then client code could catch LibraryError. This gives client code the freedom to handle the error or not. If it doesn't handle it, the exception propagates.

[programmersbook]

if allData == []:
            return False
        else:
            return allData

better:

if not allData:
    return None

return allData

[ShadyTyrant]

To Gribouillis: Your right thats a fantastic idea, thanks for the suggestion. Positive Criticism is always welcome.

To Programmers book: Your right, that statement is more logical.