rharkin
rharkin

Reputation: 23

How can i make this code more compact and easier to implement?

This is a way of searching for books in a csvfile based on data that the user provides. If they don't provide any data for a certain input such as genre, then all genres of books are used. for example if i entered: Author: - , Bookname: - , genre: Sci-Fi, then all Sci-Fi books are given, regardless of author or title.

def filltreeview(self , authorvar , booknamevar , genrevar):
    author = authorvar.get()
    bookname = booknamevar.get()
    genre = genrevar.get()
    books = []

    if genre == 'Genre':
        if author == '':
            if bookname == '':
                msg.showerror('Error' , 'Please specify some details')
            else:
                with open('Books.txt' , 'r') as csvfile:
                    reader = csv.reader(csvfile)
                    for row in reader:
                        if row == []:
                            pass
                        elif row[2] == bookname:
                            books.append([row[0],row[1],row[2],row[3]])


        else:
            with open('Books.txt' , 'r') as csvfile:
                reader = csv.reader(csvfile)    
                for row in reader:
                    if row == []:
                        pass
                    elif row[1] == author:
                        books.append([row[0],row[1],row[2],row[3]])
                print(books)
                if bookname == '':
                    pass
                else:
                    for i in books:
                        if i[2] != bookname:
                            books.remove(i)


    else:
        with open('Books.txt' , 'r') as csvfile:
            reader = csv.reader(csvfile)
            for row in reader:
                if row == []:
                    pass
                elif row[3] == genre:
                    books.append([row[0],row[1],row[2],row[3]])
                    print(books)

                    if author == '':
                        if bookname == '':
                            pass
                    else:
                        for i in books:
                            if i[1] != author:
                                books.remove(i)
                                print(books)

                    if bookname == '':
                        pass
                    else:
                        for i in books:
                            if i[2] != bookname:
                                books.remove(i)
                                print(books)

Upvotes: 0

Views: 38

Answers (2)

darthbhyrava
darthbhyrava

Reputation: 515

How about this:

def get_all_books(reader_path):
    books = []
    with open(reader_path , 'r') as csvfile:
        reader = csv.reader(csvfile)
    for row in reader:
        if row:
            books.append(row[:4])
    return books

def remove_books(books, row_num, value):
    for book in books:
        if book[row_num] is not value:
            books.remove(book)
    return books

def filltreeview(self , authorvar , booknamevar , genrevar):
    author, bookname, genre = authorvar.get(), booknamevar.get(), genrevar.get()
    if genre == 'Genre' and not author and not bookname:
        msg.showerror('Error' , 'Please specify some details')
        # a return here will avoid adding the rest of the code under 'else'
    else:
        books = get_all_books('Books.txt')
        if genre:
            books = remove_books(books, 3, genre)
        if author:
            books = remove_books(books, 1, author)
        if bookname:
            books = remove_books(books, 2, bookname)
        print(books)

Upvotes: 1

adnanmuttaleb
adnanmuttaleb

Reputation: 3604

Functions May help you here, for instance this chunk of code:

                reader = csv.reader(csvfile)
                for row in reader:
                    if row == []:
                        pass
                    elif row[i] == bookname:
                        books.append([row[x],row[y],row[z],row[w]])

Seems to be written multiple times, so you may extract it into separate function and then you can reuse it. Remember a function should do only one thing, so it can be reused with different scenarios and usecases.

Also you should have alook on singledispatch. This function should be used when you are trying to accomplish a thing, but you will do it differently based on the type of you parameter.

Upvotes: 0

Related Questions