HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Bot that selects web reviews based on spreadsheet entries

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
reviewsbotspreadsheetthatselectswebbasedentries

Problem

In my program, I prompt the user to enter a range of cities (in a spreadsheet), and then a range of review scores, and then make some decisions based on what the user has entered. Since the handling of the both user inputs (both ranges) is common to each, I have combined both into the single function getRanges() below. It works well, but I get the sense that can be improved (shortened, and with fewer if/else's). In the absence of a switch statement in Python, what are some neater approaches here?

```
# -- coding: utf-8 --
from selenium import webdriver
from selenium.webdriver.common.by import By
from selenium.webdriver.common.keys import Keys
from selenium.webdriver.support.ui import Select
from selenium.common.exceptions import NoSuchElementException
from selenium.common.exceptions import NoAlertPresentException
from selenium.common.exceptions import ElementNotVisibleException
from selenium.common.exceptions import WebDriverException
import unittest, time, re
from functions import *
from collections import OrderedDict
import csv
from openpyxl import load_workbook
from easygui import enterbox, msgbox
import traceback

class bot(unittest.TestCase):
def setUp(self):
self.wb = load_workbook("Spreadsheet.xlsx", data_only=True)
self.cities = []
self.getRanges('cities')
self.getRanges('reviews')
print 'Running...'

try:
self.driver = webdriver.Firefox()
except WebDriverException, e:
print "Unable to load profile, retrying"
try:
self.driver = webdriver.Firefox()
except WebDriverException, e:
print "Unable to load profile, retrying"
self.driver = webdriver.Firefox()
self.driver.implicitly_wait(30)
self.base_url = "https://www.google.com/ncr"
self.verificationErrors = []
self.accept_next_alert = True
self.reviewPattern = re.compile(ur'\d+')
self.niche = "plumbers+"

Solution

I will only comment on the "getting rid of the repeating ifs part". A way to avoid writing the same condition in different places is to move the check entirely into a separate piece of code. Some may refer to this as "state pattern". Below is an example:

class StateExample(object):

    class State(object):

        def __init__(self, example):
            self.example = example

        def __str__(self):
            return self.__class__.__name__

    class Cities(State):

        def query_limits(self):
            self.example.do_one_thing(self)

    class Reviews(State):

        def query_limits(self):
            self.example.do_something_else(self)

    def __init__(self):
        self.state_factories = {
            'cities': self.Cities,
            'reviews': self.Reviews
        }

    def do_one_thing(self, state):
        print 'doing one thing with %s' % state

    def do_something_else(self, state):
        print 'doing something else with %s' % state

    def interact(self, city_or_review):
        self.state = self.state_factories[city_or_review](self)

    def interact_more(self):
        self.state.query_limits()


And an example usage, to illustrate the usage:

>>> example = StateExample()
>>> example.interact('cities')
>>> example.interact_more()
doing one thing with Cities
>>> example.interact('reviews')
>>> example.interact_more()
doing something else with Reviews
>>>


You may or may not need all of the machinery here. The important part is to notice how this would be equivalent to testing repeatedly for the same condition inside interact and interact_more, which in the example above happens only once.

You can read more about this technique here: https://en.wikipedia.org/wiki/State_pattern

As an aside: it is not necessary to use classes and objects to achieve the same goal, but I want to keep this answer short.

Code Snippets

class StateExample(object):

    class State(object):

        def __init__(self, example):
            self.example = example

        def __str__(self):
            return self.__class__.__name__

    class Cities(State):

        def query_limits(self):
            self.example.do_one_thing(self)

    class Reviews(State):

        def query_limits(self):
            self.example.do_something_else(self)

    def __init__(self):
        self.state_factories = {
            'cities': self.Cities,
            'reviews': self.Reviews
        }

    def do_one_thing(self, state):
        print 'doing one thing with %s' % state

    def do_something_else(self, state):
        print 'doing something else with %s' % state

    def interact(self, city_or_review):
        self.state = self.state_factories[city_or_review](self)

    def interact_more(self):
        self.state.query_limits()
>>> example = StateExample()
>>> example.interact('cities')
>>> example.interact_more()
doing one thing with Cities
>>> example.interact('reviews')
>>> example.interact_more()
doing something else with Reviews
>>>

Context

StackExchange Code Review Q#101656, answer score: 3

Revisions (0)

No revisions yet.