1
\$\begingroup\$

I manage the Python API for the Manganelo (and Mangakalot) sites and I am not fully convinced that the code structure I currently use is the best. (https://github.com/nixonjoshua98/manganelo)

The API has various objects (MangaInfo, SearchManga, DownloadChapter) and an example of the usage are as follows.

from manganelo import SearchManga

search = SearchManga("Naruto")

search.start()

for result in search:
    print(result.title, result.url)

~ ~ ~

from manganelo import extras

search = extras.SearchMangaThread("Naruto")

search.start()  # Start the search thread

# do stuff here while we search in the background

search.wait()  # Wait for the search to finish if it hasn't already

for r in search:
    print(r)

The usage of multiple objects simply feels weird to me, and could be improved upon. The source code of this particular object is below.

class SearchManga:
    def __init__(self, query: str) -> None:
        """
        :param query: Query string to search for, we strip the 'illegal' characters ourselves.
        """

        self.query: str = query
        self.results: list = []

    def __str__(self):
        """ Return the query string which was passed in at construction. """
        return self.query

    def __enter__(self):
        """ Context manager entry point. Call .start() before we return the instance. """
        self.start()

        return self

    def __exit__(self, exc_type, exc_val, exc_tb):
        """ Context manager exit point """

    def __len__(self) -> int:
        """ Return the length of the internal results list """
        return len(self.results)

    def __getitem__(self, item):
        """ Index the internal list """
        return self.results[item]

    def __contains__(self, item):
        """ Check if an item exists in the results """
        return item in self.results

    def __iter__(self):
        """ Used in loops. Simply return results.__iter__ """
        return iter(self.results)

    def start(self):
        """ This is where the magic happens. Sends the request and extracts the information we want. """

        # Generate the URL, which includes removing 'illegal' characters
        url = self._generate_url(self.query)

        # Send the request. Can also raise an exception is the request fails.
        response = utils.send_request(url)

        # Entire page soup
        soup = BeautifulSoup(response.content, "html.parser")

        # List of the search results
        results = soup.find_all(class_="search-story-item")

        # Iterate over the results soup and extract the information we want
        for i, ele in enumerate(results):
            manga = utils.find_or_raise(ele, class_="item-img")

            title = manga.get("title", None)  # Manga title
            link = manga.get("href", None)  # Link to the manga 'homepage'

            r = MangaSearchResult(title=title, url=link)

            self.results.append(r)

    def _generate_url(self, query: str) -> str:
        """
        Generate the URL we send the request to, we remove all 'illegal' characters here from the query string.

        :param str query: THe base query string which we are searching for
        :return str: Return the formatted URL
        """
        allowed_characters: str = string.ascii_letters + string.digits + "_"

        query = "".join([char.lower() for char in query.replace(" ", "_") if char in allowed_characters])

        return "http://manganelo.com/search/" + query

The object above implements majority of the list special methods but forwards them to an internal list since I don't like inheriting from the core types.

utils

def find_or_raise(soup: BeautifulSoup, *, class_: str) -> Union[bs4.element.Tag, bs4.element.PageElement]:
    """
    Attempts to find a class inside the soup, if the tag cannot be found then raise an exception.

    :param BeautifulSoup soup:  The soup we will try to find the class <class_> in.
    :param str class_:          The class name we are searching for.
    :return:                    We return the element which is a bs4 Tag, but PyCharm marks it as a bs4 PageElement so
                                we use a Union and mark it as both types.
    """
    element = soup.find(class_=class_)

    if element is None:
        raise TagNotFound(f"Tag not found")

    return element


def send_request(url: str, *, timeout: int = 5) -> requests.Response:
    """
    Send a request to the URL provided

    :param str url: The URL which we are sending a GET request to.
    :param timeout: Optional parameter which decides how long we wait before throwing an exception
    :return: The response object
    """
    default_headers = requests.utils.default_headers()

    r = requests.get(url, stream=True, timeout=timeout, headers=default_headers)

    r.raise_for_status()

    return r

~ ~ ~

class SearchMangaThread(SearchManga):
    def __init__(self, query: str):
        super(SearchMangaThread, self).__init__(query)

        self._thread = threading.Thread(target=super(SearchMangaThread, self).start)

    def start(self):
        self._thread.start()

    def wait(self):
        self._thread.join()

    def done(self):
        return not self._thread.is_alive()

    def __enter__(self):
        self.start()

        return self

Looking for any improvements in the overall usage of the API or the code objects. I can add other object examples from the repo if requested.

Updated

class SearchManga:
    def __init__(self, query: str, *, threaded: bool = False) -> None:
        """
        :param query: Query string to search for, we strip the 'illegal' characters ourselves.
        """
        self.query: str = query

        self._response = None

        if threaded:
            self._thread = threading.Thread(target=self._start)

            self._thread.start()
        else:
            self._start()

    def _start(self):
        # Generate the URL, which includes removing 'illegal' characters
        url = self._generate_url(self.query)

        # Send the request. Can also raise an exception is the request fails.
        self._response = utils.send_request(url)

    def results(self):
        """
        This is where the magic happens. Sends the request and extracts the information we want.
        """
        if hasattr(self, "_thread") and self._thread.is_alive():
            self._thread.join()

        # Entire page soup
        soup = BeautifulSoup(self._response.content, "html.parser")

        # List of the search results
        results = soup.find_all(class_="search-story-item")

        # Iterate over the results soup and extract the information we want
        for i, ele in enumerate(results):
            manga = utils.find_or_raise(ele, class_="item-img")

            title = manga.get("title", None)  # Manga title
            link = manga.get("href", None)  # Link to the manga 'homepage'

            yield MangaSearchResult(title=title, url=link)

    @staticmethod
    def _generate_url(query: str) -> str:
        """
        Generate the URL we send the request to, we remove all 'illegal' characters here from the query string.

        :param str query: THe base query string which we are searching for
        :return str: Return the formatted URL
        """
        allowed_characters: str = string.ascii_letters + string.digits + "_"

        query = "".join([char.lower() for char in query.replace(" ", "_") if char in allowed_characters])

        return "http://manganelo.com/search/" + query
\$\endgroup\$
3
  • \$\begingroup\$ Where is APIBase defined? \$\endgroup\$
    – Reinderien
    Commented Apr 10, 2020 at 14:48
  • \$\begingroup\$ @Reinderien I actually removed the base class since posting since it only included two static methods. I have updated the code. \$\endgroup\$ Commented Apr 10, 2020 at 14:52
  • \$\begingroup\$ Please revert your latest edit and move the new code to a new question. \$\endgroup\$
    – Reinderien
    Commented Apr 10, 2020 at 18:49

1 Answer 1

0
\$\begingroup\$

In my opinion all of the machinery to make SearchManga iterable is needless. Simply change start to yield its results:

    # Iterate over the results soup and extract the information we want
    for i, ele in enumerate(results):
        manga = utils.find_or_raise(ele, class_="item-img")

        title = manga.get("title", None)  # Manga title
        link = manga.get("href", None)  # Link to the manga 'homepage'

        yield MangaSearchResult(title=title, url=link)

Anything else is overhead. If you really need this to be multi-threaded (which, given your example that simply waits, you might not), then feed a synchronized queue.Queue.

\$\endgroup\$
3
  • \$\begingroup\$ Thats a fair enough change. Think I will switch over to a generator \$\endgroup\$ Commented Apr 10, 2020 at 17:08
  • \$\begingroup\$ I added the updated version to the question \$\endgroup\$ Commented Apr 10, 2020 at 17:27
  • \$\begingroup\$ CR policy is to make a new question rather than updating the existing one. Please do that. \$\endgroup\$
    – Reinderien
    Commented Apr 10, 2020 at 18:47

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.