Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Just wondering what I did bad/could have done better. It's a simple script for pulling information from Leafly about cannabis strains. I've also added in a function to search. I am fairly new to Python. Anything is greatly appreciated. I also realize nothing is commented. I will quickly answer any questions, but it's simple enough, so hopefully you can read it.

# PYTHON 2.7.3
# Leafly API test
# STumbles

import urllib2
import json
import sys
import pprint

class Leafly:
    def __init__(self):
        self.author =   'STumbles'
        self.title  =   'Leafly API python'
        self.python_version =   '2.7.3'
    def get_info(self):
        self.search_term    =   raw_input('[*] Leafly [*]' + '\n\r'*23+': ').lower().replace(' ', '-')
        self.url    =   'http://www.leafly.com/api/details/%s' % self.search_term
        dict_a  =   json.loads(urllib2.urlopen(self.url).read())
        if (dict_a) != {}:
            return dict_a
        else:
            print 'Please type a valid strain name.'
    def specific_info(self, dict_a):
        try:
            self.container  =   []
            if dict_a['Overview']:
                self.overview   =   dict_a['Overview']
                try:
                    self.overview    =  str(self.overview)
                except UnicodeEncodeError:
                    pass
            else:
                print "could not open Overview"
            if dict_a['Rating']:
                self.rating  =   str(dict_a['Rating']) + '/10'
            else:
                print "could not open Rating"
            if dict_a['Name']:
                self.name    =   dict_a['Name']
            else:
                print "could not open Name"
            if dict_a['Abstract']:
                self.details =   dict_a['Abstract']
            else:
                print "could not open Abstract"
            if dict_a['Category']:
                self.category    =   'it is a %s' % dict_a['Category']
            else:
                print "could not open Category"
            if dict_a['Medical'][0]['Name']:
                self.medical     =   'Helps with %s' % dict_a['Medical'][0]['Name']
            else:
                print "could not open Medical"
            if dict_a['Effects'][0]['Name']:
                self.effect      =   'Makes you %s' % dict_a['Effects'][0]['Name']
            else:
                print "could not open Effects"
            if dict_a['Negative'][0]['Name']:
                self.negative    =   'Causes %s' % dict_a['Negative'][0]['Name']
            else:
                print "could not open Negative"
            try:
                self.container.append(self.name)
                self.container.append(self.rating)
                self.container.append(self.overview)
                self.container.append(self.details)
                self.container.append(self.category)
                self.container.append(self.medical)
                self.container.append(self.effect)
                self.container.append(self.negative)
                return self.container
            except AttributeError:
                pass
        except TypeError:
            pass

    def search(self):
        print '\n'*23
        self.search_url =   json.loads(urllib2.urlopen("http://www.leafly.com/api/strains").read())
        self.keyword    =   raw_input("Search: ").lower().replace(' ', '-')
        for i in self.search_url:
            for keys in xrange(1):
                if self.keyword in i['Key']:
                    print i['Key']
                else:
                    pass


print '\n'*23
flags   =   raw_input("Press 1 to search, or press 2 to get strain information\n[*] : ")
try:
    if flags == 1 or 2:
        flags   =   int(flags)
except ValueError:
    print "please enter a valid number"

leafly = Leafly()

if flags == 2:
    weed_info = leafly.get_info()
    strain_info = leafly.specific_info(weed_info)
    try:
        for i in strain_info:
            print i
    except TypeError:
        pass


if flags == 1:
    leafly.search()
else:
    pass
share|improve this question

1 Answer 1

You have two unused imports:

import pprint
import sys
  • In your methods, I notice that you are not using local variables and instead you're setting instance variables. There is no need for that, unless those variables need to be used outside of your methods.

  • There is also a bug in your code, see below. It checks if flags is equal to 1, or if 2 is True. All non-zero integer values evaluate to True, else False.

    if flags == 1 or 2: flags = int(flags)

Probably what you wanted:

if flags in (1, 2):
    flags = int(flags)
  • Another bug in your flags check is that raw_input returns a string, not an integer.

Final check:

if flags in ('1', '2'):
    flags = int(flags)
  • (EDIT 1) Since we are explicitly checking for the two correct int strings, there is no need to catch the ValueError exception that could get raised by an invalid string being passed to the int function.

  • I moved code out of the root of the file, and into a staticmethod of the Leafly class. Also, added a name == 'main' check so you can use this class in other files without running the root code.

  • You have a constructor that sets unused variables. No need to specify author and Python version as instance variables. If you really must, add them as Docstrings. Look at the final result to see how.

  • You have a giant try/catch method that catches AttributeError. This is because some of your local variables aren't set. Look at the final result to see how I've refactored it so no try/catch is required.

  • I see you only select the first entry in the lists of "medical", "effects" and "negative". Perhaps you should loop through them and add them to a list in the next version of this project. I can't infer too much from your example, so I'll leave it to you to implement.

  • You put try/catch exceptions into your code way too much. Don't do that. Rather figure out why you are getting exceptions so regularly and try deal with it. You are either doing something wrong, or the logic regarding your data and the assumptions around it are not sound. I never, ever add exception handling unless I really know I need it and that's only if I know how to handle it. Crash early, and crash often so that you can expose bugs quickly and fix them, rather than masking them.

  • I've added a sort of enumeration class to hold all your magical strings such as "Name" and "Effects", etc. It's neater, and saves you from having to find/replace all references of a string if it changes. In this example it's trivial, but for larger projects that require changes, you'll thank me.

  • I've refactored out your separate print instances of "print '\n'*23" into a ClearScreen function.

Finally, please bear in mind that with any refactoring endeavour, bugs do creep in. Bear in mind, that I have not tested the changes. Please re-test.

Final version of the code, with my changes included:

# PYTHON 2.7.3
# Leafly API test
# STumbles

import urllib2
import json



class Param(object):
    """
    Enumeration object for data structure names inside the retrieved JSON.
    """
    Overview = "Overview"
    Rating = "Rating"
    Name = "Name"
    Abstract = "Abstract"
    Category = "Category"
    Medical = "Medical"
    Effects = "Effects"
    Negative = "Negative"


def ClearScreen(lines=23):
    print '\n' * lines


class Leafly:
    """
    @author: STumbles
    @title: Leafly API python
    @python_version: 2.7.3
    """

    def get_info(self):
        print "[*] Leafly [*]"
        ClearScreen()
        self.search_term = raw_input(': ').lower().replace(' ', '-')
        self.url = 'http://www.leafly.com/api/details/%s' % self.search_term
        dict_a = json.loads(urllib2.urlopen(self.url).read())
        if (dict_a) != {}:
            return dict_a
        else:
            print 'Please type a valid strain name.'

    def specific_info(self, dict_a):
        try:
            container = []
            if dict_a['Overview']:
                overview = dict_a['Overview']
                try:
                    overview = str(overview)
                except UnicodeEncodeError:
                    pass
            else:
                print "could not open Overview"

            if dict_a[Param.Rating]:
                rating = str(dict_a[Param.Rating]) + '/10'
                container.append(rating)
            else:
                print "could not open Rating"

            if dict_a[Param.Name]:
                name = dict_a[Param.Name]
                container.append(name)
            else:
                print "could not open Name"

            if dict_a[Param.Abstract]:
                details = dict_a[Param.Abstract]
                container.append(details)
            else:
                print "could not open Abstract"

            if dict_a[Param.Category]:
                category = 'it is a %s' % dict_a[Param.Category]
                container.append(category)
            else:
                print "could not open Category"

            if dict_a[Param.Medical][0][Param.Name]:
                medical = 'Helps with %s' % dict_a[Param.Medical][0][Param.Name]
                container.append(medical)
            else:
                print "could not open Medical"

            if dict_a[Param.Effects][0][Param.Name]:
                effect = 'Makes you %s' % dict_a[Param.Effects][0][Param.Name]
                container.append(effect)
            else:
                print "could not open Effects"

            if dict_a[Param.Negative][0][Param.Name]:
                negative = 'Causes %s' % dict_a[Param.Negative][0][Param.Name]
                container.append(negative)
            else:
                print "could not open Negative"

        except TypeError:
            pass

    def search(self):
        ClearScreen()
        self.search_url = json.loads(urllib2.urlopen("http://www.leafly.com/api/strains").read())
        self.keyword = raw_input("Search: ").lower().replace(' ', '-')
        for i in self.search_url:
            for keys in xrange(1):
                if self.keyword in i['Key']:
                    print i['Key']
                else:
                    pass

    @staticmethod
    def process_user_request():
        ClearScreen()
        flags = raw_input("Press 1 to search, or press 2 to get strain information\n[*] : ")
        try:
        if flags in ('1', '2'):
            flags = int(flags)
        else:
            print "please enter a valid number"
            return

        leafly = Leafly()

        if flags == 2:
            weed_info = leafly.get_info()
            strain_info = leafly.specific_info(weed_info)
            try:
                for i in strain_info:
                    print i
            except TypeError:
                pass
        else:
            leafly.search()
        else:
            pass

if __name__ == '__main__':
    Leafly.process_user_request()
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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