2
\$\begingroup\$

I have worked on this for quite a long while, and this is my 3rd attempt after completely demolishing 3 other 200 liners, and i really need ideas on how to make my code add more functionality or efficiency, or some python coding tricks, i can implement to make it more compact. I'm still a beginning with about a month of learning.

this program is meant to run from the cmd: syntax = python "Text-Based Browser.py path" foldername - which essentially stores a folder to store the websites you visit, in shorter text, so the next time you visit the shortened site, it will be much faster. foldername is stored in Current Working Directory

it has the following inputs in cmd:
1) back: checks the previous page, using stack implementation
2) exit: exits out the program
3) an Url: checks if the url is valid, and if it is valid then checks if a connection can be made, if a connection can be made, then it retrieves the html of the website, and then parses for the text based tags, and changes the "a" tags to the color blue using the colorama module, and writes the text in a file in the foldername. If the url is incorrect, prints a prompt

import sys
import os
import requests
from bs4 import BeautifulSoup
import colorama


dir_name = ''
if len(sys.argv) == 2:
    dir_name = sys.argv[1]
    if not os.path.exists(dir_name):
        os.mkdir(dir_name)
path = dir_name + '/' if dir_name else ''
colorama.init(autoreset=True)
protocol = 'https://'


def read_tab(file, directory):
    with open(directory + file, encoding='utf-8') as f:
        return f.readlines()


def write_tab(file, directory, text_list):
    tab = file.split(protocol)[-1]
    tab = '.'.join(tab.split('.')[:-1])
    with open(directory + tab, 'w', encoding='utf-8') as f:
        f.writelines(text_list)
    return tab


def request_page(page):
    if '.' not in url:
        raise Exception
    if protocol not in page:
        page = protocol + page
    response = requests.get(page, headers={'user-agent': 'Mozilla/5.0'}, timeout=5)
    if not response:
        raise Exception

    soup = BeautifulSoup(response.content, 'lxml')
    tag_set = {'p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'a', 'ul', 'ol', 'li'}
    text_list = []
    for string in soup.strings:
        parents = {parent.name for parent in string.parents}
        if tag_set & parents:
            txt = '\033[34m' + str(string).strip() if 'a' in parents else str(string).strip()
            _ = text_list.append(txt + '\n') if len(txt) > 2 and not txt.startswith(('$', '.', '@')) else 0
    return text_list


history_stack = []
last_page = None
while True:
    content = []
    url = input("Text Based Webbrowser\nEnter Url/Action: ").strip()
    if url == 'back':
        if len(history_stack) == 0:
            continue
        url = history_stack.pop()
    else:
        _ = history_stack.append(last_page) if last_page else None
        if url == 'exit':
            break
    # Request URL and write Tab:
    try:
        content = request_page(url)
    except Exception:
        # Read Tab if error of requesting (this is tab probably):
        try:
            content = read_tab(url, path)
        except Exception:
            print('Error: Incorrect URL or Tab, enter again')
            continue
        else:
            last_page = url
    else:
        last_page = write_tab(url, path, content)
    finally:
        _ = [print(line, end='') for line in content]
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Re. please go easy on me - There's no expectation for reviewers to hold back reviews so far as they're on-topic and constructive; but policy requires that they be nice - meta.stackexchange.com/questions/240839/… \$\endgroup\$
    – Reinderien
    Apr 16 '20 at 14:27
  • \$\begingroup\$ I'm not accusing you of having caused a misdemeanor - I'm saying that the extent to which reviewers answering this question go easy on you is that they be nice. :) \$\endgroup\$
    – Reinderien
    Apr 16 '20 at 15:53
  • \$\begingroup\$ oh that was oblivious of me, sorry for not getting your point. There are quite the rumors, that stackexchange users are merciless with their reviews, it appears it's only a rumor and nothing else. \$\endgroup\$
    – DeltaHaxor
    Apr 16 '20 at 15:56
  • \$\begingroup\$ StackOverflow can be pretty cutting sometimes, particularly for people who are not well-versed in the expected question format. I anecdotally find CodeReview to be more forgiving. \$\endgroup\$
    – Reinderien
    Apr 16 '20 at 15:57
2
\$\begingroup\$

Use argparse

Argparse will give you cleaner code and a friendlier CLI. It will simplify much of this blob:

dir_name = ''
if len(sys.argv) == 2:
    dir_name = sys.argv[1]
    if not os.path.exists(dir_name):
        os.mkdir(dir_name)
path = dir_name + '/' if dir_name else ''

Also, for path handling and directory creation consider the use of pathlib.Path.

write_path

You're passing an URL into what you've called the file parameter, so that's confusing. Also, it appears that you're manually parsing that URL using split when you're better off using urllib.parse.

Ternary

This:

_ = text_list.append(txt + '\n') if len(txt) > 2 and not txt.startswith(('$', '.', '@')) else 0

should be expanded out into a plain if:

if len(txt) > 2 and not txt.startswith(('$', '.', '@')):
    text_list.append(txt + '\n')

Similarly, this:

    _ = [print(line, end='') for line in content]

is doing a comprehension and throwing away the result. Instead,

print(''.join(content))

Error handling

response = requests.get(page, headers={'user-agent': 'Mozilla/5.0'}, timeout=5)
if not response:
    raise Exception

First, never raise a base Exception - you want to raise something more specific. That said, in this case, replace your if with a response.raise_for_status().

In a similar case,

if '.' not in url:
    raise Exception

should instead be raising something like ValueError(f'Malformed URL {url}'). It's also odd that this is referring to the global url variable instead of the page argument. Finally: to do this kind of validation (not sure why it matters that there's a dot in the URL), you should be using urllib.parse.

Global code

Pull everything from history_stack = [] onwards into one or more functions.

\$\endgroup\$

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.