1
\$\begingroup\$

I'm new to programming. I wrote this following console based app in Python. It's an English dictionary app. If the user inputs the desired word that he/she wishes to search for, then this app should be able to display the definition, example(used in a sentence), synonyms, origins, phonetics, parts of speech of the searched word. It's kinda like PyDictionary, which is a package; on the other hand, it's a full-functional(so far...) app and it uses different online resources. This app is depended on BeautifulSoup and requests packages. I hope someone can point me to make it better and shorter and if there's any mistake in the code.

import requests as r
from bs4 import BeautifulSoup as BS


def server_one(url):  # Oxford online dictionary
    """ Server_one scrapping algorithm to get essential information regarding the searched word """

    soup = get_soup(url)

    try:
        definition = soup.select_one('.ind').get_text(strip=True)  # Scraps first definition
    except AttributeError:
        definition = 'NOT FOUNT'

    try:
        example = soup.select_one('.exg').get_text(strip=True)  # Scraps first example
    except AttributeError:
        example = 'NOT FOUNT'

    try:
        parts_of_speech = soup.select_one('.pos').get_text(strip=True).capitalize()  # Gets the POS
    except AttributeError:
        parts_of_speech = 'NOT FOUNT'

    try:
        synonyms = soup.select_one('.exs').get_text(strip=True).split(', ')  # Collects all the synonyms
    except AttributeError:
        synonyms = 'NOT FOUNT'

    if 'NOT FOUNT' in synonyms:
        synonyms_tobe_sent = ''
    else:
        synonyms_tobe_sent = [cap.capitalize() for cap in synonyms]  # Capitalizes all the elements in the list

    try:
        origins = []
        for ori in soup.select('.senseInnerWrapper'):  # Scraps all the origins in a list
            if len(ori.text) < 400:
                origins.append(ori.text)
    except AttributeError:
        origins = 'NOT FOUNT'

    try:
        phonetics = soup.select_one('.phoneticspelling').get_text(strip=True)  # Gets the phonetics
    except AttributeError:
        phonetics = 'NOT FOUNT'

    try:
        next_definitions = []
        for tag in soup.select('.ind'):  # Gathers all possible definitions of the searched word
            next_definitions.append(tag.text)
        try:
            next_definitions.pop(0)  # Removes the first definition since it's already used before
        except IndexError:
            pass
    except AttributeError:
        next_definitions = 'NOT FOUNT'

    if example != 'NOT FOUNT':
        example_tobe_sent = example[1:-1].capitalize() + '.'  # Removes colons(') from the string and adds a (.)
    else:
        example_tobe_sent = 'NOT FOUNT'

    return definition, example_tobe_sent, next_definitions, parts_of_speech, synonyms_tobe_sent, origins, phonetics


def search_word():
    """ This function returns the searched word in lower format """

    word = input('Word that you wish to search\n>>> ').lower()
    return word


def url_server_one():
    """ A function to get correct URL for the server_one """

    w = search_word()
    new_w = w.replace(' ', '_')  # If a word has space then it gets _ instead, e.g. 'look up' turns into 'look_up'
    if ' ' in w:
        url = 'https://en.oxforddictionaries.com/definition/%s' % new_w  # URL with underscore(_)
    else:
        url = 'https://en.oxforddictionaries.com/definition/%s' % w  # URL without underscore
    data = (url, w)  # To make two variables as a tuple
    return data


def get_soup(url):
    """ This returns the bs4 soup object that will be used for scrapping """

    source_code = r.get(url)
    plain_text = source_code.text
    soup_data = BS(plain_text, 'html.parser')
    return soup_data


def display_dict():
    """ A function to display all the collected information in a desired format, currently supports only server_one """

    url, title = url_server_one()
    def_, exam, next_defs, part_sp, synos, ori, phonet = server_one(url)

    print('\nDefinition of %s:\n%s\n' % (title.capitalize(), def_))
    print('Parts of Speech:\n%s\n' % part_sp)
    print('Phonetics of %s:\n%s\n' % (title.capitalize(), phonet))
    print('Example of %s as follows:\n%s\n' % (title.capitalize(), exam))

    print('Synonyms:')
    if synos != '' or synos != []:
        print(', '.join(synos))
        print()
    if len(synos) < 1:
        print('NOT FOUND\n')

    print('Origins:')
    if ori != '' or ori != []:
        for i in ori:
            if i != '' or i != 'NOT FOUNT':
                print(i, sep=' ')
            print()
    if len(ori) < 1:
        print('NOT FOUND\n')

    print('Some other definition(s) of %s:' % title.capitalize())
    if next_defs != '' or next_defs != []:
        for i in next_defs:
            if i != '' or i != 'NOT FOUNT':
                print('* ' + i.capitalize() + '\n')
    if len(next_defs) < 1:
        print('NOT FOUND\n')


def try_():
    """ Part of repeat() function that returns integer value """

    while True:
        try:
            x = int(input('Press 1 to search again\n'
                      'Press 2 to quit'))
            if x in {1, 2}:
                return x
            print('Either press 1 or 2')
        except ValueError:
            print('Enter an integer')


def repeat():
    """ A function to repeat the process of searching over again and quit from the app """

    _try = try_()
    if _try == 1:
        display_dict()
    else:
        quit()


def main():
    print('Welcome to PyEngDict V1.0 by AJ\n')
    display_dict()  # Initially launches the app's search option
    while True:
        repeat()

if __name__ == '__main__':
    main()

A screenshot of the app

\$\endgroup\$
  • \$\begingroup\$ You have multiple trys that can be combined into a for loop; also "found" is spelled "found", not "fount" (minor). \$\endgroup\$ – Barry Carter Sep 17 '17 at 14:32
  • \$\begingroup\$ @BarryCarter thanks, man. Instead of saying could u plz illustrate how it's done? I hope i know how to spell found, but that's not an issue here. \$\endgroup\$ – A J Sep 17 '17 at 14:51
  • \$\begingroup\$ I'm too lazy to actually show you, but consider:definition = soup.select_one('.ind'). You do this over and over again with ind replaced with different strings. That highly suggests doing something like definition = soup.select_one(i), where i is a loop variable. \$\endgroup\$ – Barry Carter Sep 17 '17 at 14:53
  • \$\begingroup\$ @BarryCarter thanks, for not being too lazy while explaining this.. got some ideas.. \$\endgroup\$ – A J Sep 17 '17 at 15:00
3
\$\begingroup\$

Here are some of the things I would work on:

  • variable naming - try to name your variables in a descriptive way - variable names like i (note that it is not used as an index variable but as a next origin or definition) or ori, part_sp are not clear enough - it takes time to understand what are they used for. Making variables shorter is not the right motivation when it hurts readability.
  • if ori != '' or ori != []: could be replaced with if ori:, if next_defs != '' or next_defs != []: can be replaced with if next_defs:
  • I am not sure you actually need both w and new_w inside the url_server_one() function - you can replace a space with an undescore for every search string:

    def url_server_one():
        """A function to get correct URL for the server_one."""
    
        word = search_word().replace(' ', '_')
        url = 'https://en.oxforddictionaries.com/definition/' + word
    
        return (url, word)
    
  • handling AttributeError in this part does not make sense - it would not be thrown by .select() or .text since .select() would return an empty list if no matching tags found and a list of Tag instances which all would have text attribute. In other words, AttributeError would not be thrown:

    try:
        origins = []
        for ori in soup.select('.senseInnerWrapper'):  # Scraps all the origins in a list
            if len(ori.text) < 400:
                origins.append(ori.text)
    except AttributeError:
        origins = 'NOT FOUNT' 
    

    (same goes for the block where you collect next_definitions)

  • you can use a list comprehension to define origins list:

    origins = [ori.text for ori in soup.select('.senseInnerWrapper') if len(ori.text) < 400]
    
  • you can use a multi-line string instead of multiple print calls with new-line characters

There are also these try and except AttributeError: repetitive blocks. There are multiple ways to address that. One is to switch to a dictionary-based approach, first define a mapping between field names and CSS selectors, something like:

selectors = {
    'definition': '.ind',
    'example': '.exg',
    'parts_of_speech': '.pos',
    'phoneticspelling': '.phoneticspelling',
    # ...
}

field_values = {}
for field, selector in selectors.items():
    tag = soup.select_one(selector)
    field_values[field] = tag.get_text() if tag is not None else 'NOT FOUND'
\$\endgroup\$
  • \$\begingroup\$ thanks, for ur awesome feedback. Learnt some cool tricks from it. If u get a chance could u review first two functions of my revised code and give a feedback? \$\endgroup\$ – A J Sep 18 '17 at 12:43
  • 1
    \$\begingroup\$ @AJ please do not change the original code in the question and, instead, post a separate question as a next round of a code review. Thanks! \$\endgroup\$ – alecxe Sep 18 '17 at 12:59
  • \$\begingroup\$ okay, did't know i couldn't do that. Btw, did u have a look at my deleted code? @alecxe \$\endgroup\$ – A J Sep 18 '17 at 13:05
  • 1
    \$\begingroup\$ @AJ well, first of all, the revision is not lost (you can find it here). This is just how things work on the Code Review site (reference). Please post the modified code in a separate thread and I'll have a closer look there. Thanks for understanding! \$\endgroup\$ – alecxe Sep 18 '17 at 13:34

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.