3
\$\begingroup\$

Here is the script that I would like reviewed for the following:

  1. Best practices and design pattern usage
  2. Correctness in unanticipated cases
  3. Better dictionary access

The script is made to store (IPFS) Interplanetary File System hash objects. If you would like to know more check out https://ipfs.io

Script:

#!/usr/bin/python
# PHT - Personal Hash Tracker
#
import json
import os

hashjson_global = "/Users/troywilson/testing/pht/hash.json"

choice = raw_input("What do you want to do? \n a)Add a new IPFS hash\n s)Seach stored hashes\n d)Delete stored hash\n >>")

if choice == 'a':
    # Add a new hash.
    description = raw_input('Enter hash description: ')
    new_hash_val = raw_input('Enter IPFS hash: ')
    new_url_val = raw_input('Enter URL: ')
    entry = {new_hash_val: {'description': description, 'url': new_url_val}}

    # search existing hash listings here
    if new_hash_val not in data['hashlist']:
    # append JSON file with new entry
        data['hashlist'].update(entry) #must do update since it's a dictionary
        with open(hashjson_global, 'w') as file:
            json.dump(data, file, sort_keys = True, indent = 4, ensure_ascii = False)
        print('IPFS Hash Added.')
        pass
    else:
        print('Hash exist!')

elif choice == 's':
    # Search the current desciptions.
    searchTerm = raw_input('Enter search term: ')
    with open(hashjson_global, 'r') as file:
        data = json.load(file)
        hashlist = data['hashlist']
    # build dictionary map and search for description value
    d = {v['description']: h for h, v in hashlist.items()}
    print d.get(searchTerm, 'Not Found')


elif choice == 'd':
    # Search the current descriptions and delete entry.
    del_hash = raw_input('Hash to delete: ')
    with open(hashjson_global, 'r') as file:
        data = json.load(file)
    del data['hashlist'][del_hash]
    with open('hashjson', 'w') as file:
            json.dump(data, file, sort_keys = True, indent = 4, ensure_ascii = False)
    print ('Hash removed')

Example JSON file:

{
"hashlist": {
"QmVZATT8jWo6ncQM3kwBrGXBjuKfifvrE": {
    "description": "Test Video",
    "url": ""
},
"QmVqpEomPZU8cpNezxZHG2oc3xQi61P2n": {
    "description": "Cat Photo",
    "url": ""
},
"QmYdWb4CdFqWGYnPA7V12bX7hf2zxv64AG": {
    "description": "test.co",
    "url": ""
}
}
}%
\$\endgroup\$
  • \$\begingroup\$ Is that percent sign supposed to be at the end of the JSON file? \$\endgroup\$ – Arnav Borborah Feb 23 '18 at 23:06
  • \$\begingroup\$ The JSON file is generated. I don't know for sure if the % should be there but it does not affect how it works. \$\endgroup\$ – Troy Wilson Feb 23 '18 at 23:07
  • 1
    \$\begingroup\$ It looks like data is undefined if the user chooses "a" as the first choice. You are missing the reading of the data file there. \$\endgroup\$ – Graipher Feb 24 '18 at 21:19
1
\$\begingroup\$

I would make an object that acts like a dict and that automatically writes any changes to disk, similar to the one I wrote in this answer.

import os
import json

class PersonalHashTracker(dict):

    def __init__(self, filename):
        self.filename = filename
        if os.path.isfile(filename):
            with open(filename) as f:
                # use super here to avoid unnecessary write
                super(PersonalHashTracker, self).update(json.load(f) or {})

    def write(self):
        with open(self.filename, "w") as f:
            json.dump(self, f)

    def __setitem__(self, key, value):
        super(PersonalHashTracker, self).__setitem__(key, value)
        self.write()

    def __delitem__(self, key):
        super(PersonalHashTracker, self).__delitem__(key)
        self.write()

    def update(self, d, **kwargs):
        super(PersonalHashTracker, self).update(d, **kwargs)
        self.write()

You can then use it like this:

MENU = """What do you want to do?
a)Add a new IPFS hash
s)Seach stored hashes
d)Delete stored hash
q)Quit
>>"""

if __name__ == "__main__":
    hash_store = PersonalHashTracker("/Users/troywilson/testing/pht/hash.json")

    while True:
        choice = raw_input(MENU)
        if choice == 'a':
            # Add a new hash.
            description = raw_input('Enter hash description: ')
            new_hash_val = raw_input('Enter IPFS hash: ')
            new_url_val = raw_input('Enter URL: ')

            if new_hash_val not in hash_store:
                hash_store[new_hash_val] = {'description': description,
                                            'url': new_url_val}
            else:
                print 'Hash exists!'
        elif choice == 's':
            # Search the current descriptions.
            search_term = raw_input('Enter search term: ')
            descriptions = {v['description']: h for h, v in hash_store.items()}
            print descriptions.get(search_term, 'Not Found')
        elif choice == 'd':
            # Search the current descriptions and delete entry.
            del_hash = raw_input('Hash to delete: ')
            del hash_store[del_hash]
            print 'Hash removed'
        else:
            print 'Exiting'
            break
\$\endgroup\$
  • \$\begingroup\$ This looks amazing! I just put the two code blocks together and I'm working on figuring out how it works. It does have an error NameError: global name 'Config' is not defined. @Graipher \$\endgroup\$ – Troy Wilson Feb 27 '18 at 3:27
  • \$\begingroup\$ @TroyWilson Oops, forgot to rename the class name in the calls to super. See edited code \$\endgroup\$ – Graipher Feb 27 '18 at 6:38
  • \$\begingroup\$ It looks like the add and delete don't actually call the class to preform the action. \$\endgroup\$ – Troy Wilson Mar 3 '18 at 23:18
  • \$\begingroup\$ PersonalHashTracker.__delitem__(hash_store['hashlist'][del_hash]) throws a KeyError when added to the delete option. I don't think this works. @Graipher \$\endgroup\$ – Troy Wilson Mar 4 '18 at 6:52
  • \$\begingroup\$ @TroyWilson You are right. There are a few typos (data -> hash_store), but even then it does not work. The reason is the nested dictionary hash_list inside hash_store. Since all operations are on that, the class methods are never called. The easiest fix would be making it a not nested datastructure, so saving all keys directly in the top dictionary. \$\endgroup\$ – Graipher Mar 4 '18 at 8:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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