0
\$\begingroup\$

Do you have any suggestions to improve or extend this script? Any ideas for removing or adding comments/docstrings?

import dbm
import subprocess
import time
subprocess.call("clear")
try:
    # Tries to import cPickle, a faster implementation of pickle.
    import cPickle as pickle
except ImportError:
    import pickle

class PickleDB:
    def open_db(self):
        """Gets and opens the database"""
        self.db_name = input("Enter a database name\n")
        # Makes sure the database name is compatible
        db_parts = self.db_name.split(".")
        self.db_name = db_parts[0]
        self.db = dbm.open(self.db_name, "n")

    def get_data(self):
        """Gets data to write into the database"""
        subprocess.call("clear")
        self.raw_data = input("Enter your raw data\n")
        # Pickles data
        self.pickled_data = pickle.dumps(self.raw_data)

    def write_to_db(self):
        """Writes data into database"""
        # Logs time
        self.start = time.time()
        # Creates keys
        self.db["raw"] = self.raw_data
        self.db["pickled"] = self.pickled_data

    def close_db(self):
        """Closes database"""
        self.db.close()
        # Logs end time
        self.end = time.time()

    def info(self):
        """Prints out info about the database"""
        subprocess.call("clear")
        print("Load data from database using dbm")
        print("Keys: raw\tpickled")
        # Prints total time
        print("Speed:", self.end - self.start, "seconds")

# Function calls
pickled_db = PickleDB()
pickled_db.open_db()
pickled_db.get_data()
pickled_db.write_to_db()
pickled_db.close_db()
pickled_db.info()
\$\endgroup\$
0

2 Answers 2

3
\$\begingroup\$
import dbm
import subprocess
import time
subprocess.call("clear")

This isn't cross-platform. Sadly, there isn't really a cross-platform alternative

try:
    # Tries to import cPickle, a faster implementation of pickle.
    import cPickle as pickle
except ImportError:
    import pickle

class PickleDB:
    def open_db(self):
        """Gets and opens the database"""
        self.db_name = input("Enter a database name\n")

You really shouldn't call input inside of a work class. The code responsible for User I/O should be separate from that doing any actual work.

        # Makes sure the database name is compatible
        db_parts = self.db_name.split(".")
        self.db_name = db_parts[0]

Ok, why are you ignoring part of the name if its not good? Shouldn't you complain to the user and ask for a new name? Also, I'd avoid assigning to self.db_name only the replace it again a bit later. Since you don't use it outside this function, I'd probably just have a local variable

        self.db = dbm.open(self.db_name, "n")

    def get_data(self):
        """Gets data to write into the database"""
        subprocess.call("clear")
        self.raw_data = input("Enter your raw data\n")
        # Pickles data

This comment is a little obvious, I'd delete it

        self.pickled_data = pickle.dumps(self.raw_data)

I'd expect a function that gets data to return it to the caller. Generally, functions that drop state onto the class is a sign of poor design. Methods should usually communicate by parameters and return values not object storage.

    def write_to_db(self):
        """Writes data into database"""
        # Logs time
        self.start = time.time()

Collecting timing information isn't something that's usually part of the actual class doing the work. Usually you'd time the class from the outside.

        # Creates keys
        self.db["raw"] = self.raw_data
        self.db["pickled"] = self.pickled_data

    def close_db(self):
        """Closes database"""
        self.db.close()
        # Logs end time
        self.end = time.time()


    def info(self):
        """Prints out info about the database"""
        subprocess.call("clear")
        print("Load data from database using dbm")
        print("Keys: raw\tpickled")
        # Prints total time
        print("Speed:", self.end - self.start, "seconds")

As noted, output and timing is usually something you want to do outside of the work class itself.

# Function calls
pickled_db = PickleDB()
pickled_db.open_db()
pickled_db.get_data()
pickled_db.write_to_db()
pickled_db.close_db()
pickled_db.info()

Here's the overall thing about your code. You are using a class as a group of functions and treating the object storage like a place to put global variables. You aren't making effective use of the class. In this case, you'd be better off to use a collection of functions than a class.

\$\endgroup\$
0
1
\$\begingroup\$

I think Winston Ewert summed up quite well what is wrong with the code. You should replace all that is after the line # Function calls by

if __name__ == "__main__":
    pickle_db = PickleDB(input("Enter a database name\n"))
    pickle_db.write(input("Enter your raw data\n"))
    pickle_db.close()
    print("Speed:{0} seconds".format(pickle_db.get_total_time())

and build your class from that snippet of code (put the open function in the init, for example)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.