vintagedave / fontimize Goto Github PK

Hi,

I randomly came across your blog post on this library and thought I'd take a look at the code. I noticed you have an accidental O(n^2) string concatenation. In a few places in your code, you're appending lists of strings. This is a problem as it creates new intermediate strings in each iteration. Here is one such example:

A more efficient way to write the above would be something like the following as it creates a single new string object:

text = ''.join(BeautifulSoup(html, 'html.parser').get_text() for html in html_contents)

The main function optimise_fonts and the other exposed functions using it return dict[str, typing.Any]. This is inconvenient for the users of the library as they will have no autocomplete and type safety. There are two possible ways you can solve it. I would recommend 1 here, although it will break backwards compatibility. You can use 2 if you really prefer dictionaries and don't want to break the backwards compatibility.

1. Using a dataclass:

Returning a suitable dataclass instead will allow users to access the values with dot notation:

from dataclasses import dataclass, field

@dataclass
class FontResult:
    css: set[str] = field(default_factory=set)
    fonts: dict[str, str] = field(default_factory=dict)
    chars: set[str] = field(default_factory=set)
    uranges: str = ''

Then you can return an instance of this instead of your dictionary:

font_res = FontResult()
...
font_res.chars = characters  # etc
...
return font_res

2. Using TypedDict:

Return a typed dictionary. The code would be something along these lines:

from typing import TypedDict


class FontResult(TypedDict):
    css: set[str]
    fonts: dict[str, str]
    chars: set[str]
    uranges: str


def optimise_fonts(...) -> FontResult:
    font_res = {'css': set(), 'fonts': {}, 'chars': set(), 'uranges': ''}
    ...
    return font_res