1
\$\begingroup\$

I made this piece of code as first step for a bigger project to iterate and manage bookmarks: open, rename, place in new folder structure, or delete each one of them. This first step is a parser to get a useful json file with the structure and data of each bookmark and folder. I've usex regex instead of beautifulsoup to get somehow legacy code, it's not that hard for the task (but maybe not the best implementation, I just know It works for me).

Im starting to publish my code and I am a little lost, I don't get to know why I can't draw feedback, and my main guess is that I'm doing something so wrong people don't even bother. My concerns are not about best practices but quite the opposite: which bad practices do you see here? My main concerns are following PEP8 (python style), having a clear and useful functionality, and clear comments to let the code explain itself.

I came here because I know that asking the right questions this community has really good answers based on experience and knowledge, and while I'm building up my own It's really nice when you guys shine some light into the darkness.

Without further ado, here is the code:

import re
import json

class BookmarkParser():

    def __init__(self, infile):
        # Read input file, lines to list, strip each line.
        # Utf8 because spanish
        self.file_lines = []

        with open(infile, encoding='utf8') as f:

            self.file_lines = f.read().splitlines()

        for i in range(len(self.file_lines)):

            self.file_lines[i] = self.file_lines[i].strip()


        # self.tree: dict with the whole structure
        # root: key that holds all the structure in its value, it's used
        # to have a root directory that is not the tree itself
        self.tree = {'root':{}}

        # list to store the path of the current directory through the iteration,
        # used by change_folder
        self.path = ['root']

        # reference to the tree used by change_folder, stores current folder on iteration
        self.folder = self.tree

        self.change_folder()
        self.iterate()
        self.save()

    def change_folder(self):
        # change the folder reference to the last folder in path
        self.folder = self.tree

        for key in self.path:

            self.folder = self.folder[key]

    def iterate(self):
        # Iterate html lines to parse structure from <DT H3 and </Dl, and data from <DT H3 and <DT A
        for line in self.file_lines:

            # If the first tag is DT
            if line[1:3] == 'DT':

                # And if the second tag is A
                if line[5] == 'A':

                    # Its a link: get its parameters and append to current folder
                    name = re.findall('(?<=>).*?(?=<)', line)[1]
                    href = re.search('(?<=HREF=").*?(?=")', line).group()
                    add_date = re.search('(?<=ADD_DATE=").*?(?=")', line).group()
                    icono = re.search('(?<=ICON=").*?(?=")', line)
                    icon=''

                    if icono:

                        # Not all links have one
                        icon = icono.group()

                    info = {
                        'url':href,
                        'add_date':add_date,
                        'icon':icon
                    }

                    self.folder.update({name:info})

                # If the second tag is H3
                elif line[5:7] == 'H3':

                    # Its a folder: get its parameters, append to current folder,
                    # create self folder and set current folder to self
                    name = re.findall('(?<=>).*?(?=<)', line)[1]
                    last_modified = re.search('(?<=LAST_MODIFIED=").*?(?=")', line).group()
                    add_date = re.search('(?<=ADD_DATE=").*?(?=")', line).group()

                    info = {
                        'add_date':add_date,
                        'last_modified':last_modified
                    }

                    self.folder.update({name:{'meta':info}})
                    self.path.append(name)
                    self.change_folder()

            # If tag is /DL we closed current folder: pop it from self.path and change folder
            elif line[1:4] == '/DL':

                self.path.pop()
                self.change_folder()

    def save(self):
        # Save the tree dict to a json file. Using utf8 here because spanish.
        with open('parsed_bookmarks.json','w',encoding='utf8') as outfile:

            json.dump(self.tree, outfile, indent=4, ensure_ascii=False)

if __name__ == '__main__':

    parser = BookmarkParser('bookmarks_chrome.html')
\$\endgroup\$
2
  • \$\begingroup\$ Please don't modify the code in the question after having received an answer. This edit (partially) invalidated the answer, so I have rolled back your last edit. Have a look at What should I do when someone answers my question?. You can always ask a follow-up question with updated code (it is customary to link to the first question in this case, for reference). \$\endgroup\$
    – Graipher
    Commented Dec 9, 2018 at 18:20
  • 1
    \$\begingroup\$ Thx! Neat to know, sorry for the mistake. \$\endgroup\$
    – Pitrow P.
    Commented Dec 9, 2018 at 19:11

1 Answer 1

1
\$\begingroup\$

As you said, regex is not meant for HTML parsing. But as you are aware of it and it's a conscious choice, parsing a well-defined tree is, I guess, OK. The code is easy to read and follow. One improvement would be to break out the actual tag handling (the content of the ifs) from iterate to separate functions, like handle_link, handle_folder, etc. That way, it's even easier to scan the code for functionality without getting into the nitty gritty details.

Opening, reading, and stripping the file could all be done inside the with statement. Keep closely related stuff together, easier to follow.

The change_folder function is too complex. To get items from the end of a list, just use a negative index. The last item is thus [-1].

\$\endgroup\$
1
  • \$\begingroup\$ Thanks! I struggled with the change directory one completely forgetting the negative index thing, oh my! I'm working on a new approach reading the bookmarks file without parsing the export, but this little script really helped me to learn and I really appreciate your feedback \$\endgroup\$
    – Pitrow P.
    Commented Dec 9, 2018 at 16:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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