tryexceptpass / sofi

an OS agnostic UI module for Python
MIT License
387 stars 49 forks source link

Refactor __str__() implementation #73

Closed Michael-F-Bryan closed 8 years ago

Michael-F-Bryan commented 8 years ago

I was looking through some of the source code for various elements and it seems like you're doing loads of repetitive work. For example, here's the __str__() for the div element.

def __str__(self):
        output = [ "<div" ]

        if self.ident:
            output.append(" id=\"")
            output.append(self.ident)
            output.append("\"")

        if self.cl:
            output.append(" class=\"")
            output.append(self.cl)
            output.append("\"")

        if self.style:
            output.append(" style=\"")
            output.append(self.style)
            output.append("\"")

        if self.attrs:
            for k in self.attrs.keys():
                output.append(' ' + k + '="' + self.attrs[k] + '"')

        output.append(">")

You could probably make your life a bit easier by using getattr() and friends. So something like this:

def __str__(self):
    output = ['<div']

    attributes = [
           # tuple of (attr_name_in_self, attr_name_in_html)
            ('ident', 'id'),
            ('cl', 'class'),
            ('style', 'style'),
    ]

    for name, as_html in attributes:
        # Grab the value of the `name` attribute from `self`
        value = getattr(self, name)
        if value:
            output.append(' {}="{}"'.format(as_html, value))

    output.append('>')

It looks pretty complicated, but I'm sure it wouldn't be difficult to extend/abstract out something like so the generation of most html elements can be done automatically.

This is an awesome project by the way, I only came across it the other day but I've already got plenty of ideas for places to use it. Keep up the great work!

tryexceptpass commented 8 years ago

I agree, there's a lot of repetitive work, after I had the first dozen or so widgets I started to look into refactoring, especially compositing some widgets on top of others, and I ran into a few issues with slight differences in how the HTML needed to be put together that made things a less legible, so for the moment I've chosen to flesh it all out and do a refactoring sweep once things are further along.

I like what you're suggesting, may make sense to stick that in the Element class to have a default way of generating the HTML and then overriding it wherever it makes sense to do something different. I'll stick it in the queue.

ugy commented 8 years ago

You could even use the beautifulsoup module to generate all the text for you. https://www.crummy.com/software/BeautifulSoup/

ugy commented 8 years ago

I thought of a new Element like the one below but it would be to code something already existing:

class Element(object):
    """Base HTML tag element"""

    def __init__(self, name, cls=None, ident=None, style=None, paired=True, **attrs):
        """Create a base element where cls is a space separated class attribute,
           ident is the element id and style is a CSS style string"""

        self.name = name

        if cls:
            self.cls = set(cls.split())
        else:
            self.cls = set()

        self.ident = ident
        self.style = style
        self.attrs = attrs
        self.paired = paired

        if paired:
            self.children = list()

    def __repr__(self):
        return self.__str__()

    def __str__(self):

        output = ['<' + self.name]
        if self.ident:
            output.append(' id="' + self.ident + '"')
        if self.cls:
            output.append(' class="' + ' '.join(self.cls) + '"')
        if self.style:
            output.append(' style="' + self.style + '"')
        if self.attrs:
            output += [' ' + k + '="' + v + '"' for k, v in self.attrs.items()]

        output.append('>')

        if self.paired:

            output += [str(child) for child in self.children]
            output.append('</' + self.name + '>')

        return ''.join(output)

    def addclass(self, cls):
        """Add one or multiple classes to this element"""

        self.cls |= set(cls.split())

    def removeclass(self, cls):
        """Remove one or multiple classes from this element"""

        self.cls -= set(cls.split())

    def addchild(self, elem):
        """Add a child element to this tag"""

        self.children.append(elem)

    def removechild(self, elem):

        self.children.remove(elem)

With this class you could implement Select like this:

class Select(Element):
    """Implements a boostrap select tag"""

    def __init__(self, **kwargs):
        super().__init__(name='select', **kwargs)

    def addOption(self, opt):

        # opt is an Option instance
        if isinstance(opt, Option):
            self.addchild(opt)
            return opt

        # opt is string
        result = opt.split()

        for option in result:
            self.addchild(Option(option))

        return result

    def removeOption(self, opt):

        if isinstance(opt, Option):
            self.removechild(opt)
        else:

            opt_set = set(opt.split())

            self.children = [
                c for c in self.children
                if not isinstance(c, Option) or c.name not in opt_set
            ]

    def setdefault(self, opt, default=True):

        for child in self.children:
            if isinstance(child, Option) and child.value == opt:
                child.setDefault(default)

class Option(Element):
    """Implements <option> tag for select"""

    def __init__(self, value, **kwargs):
        super().__init__(name='option', **kwargs)
        self.value = value
        self.addchild(value)

    def setDefault(self, default=True):
        if default:
            self.attrs['selected'] = 'selected'
        elif 'selected' in self.attrs:
            del self.attrs['selected']
Michael-F-Bryan commented 8 years ago

@ugy, I like the idea of leveraging BeautifulSoup to do all the html construction. I've used it a lot for web scraping and html manipulation and it gives you a really nice API to work with.

Automating the html construction sounds like a lot nicer way to do things than manual string manipulation.

tryexceptpass commented 8 years ago

On BeautifulSoup. It's a good idea, haven't used it in a while, I'll look into it a bit more. However, I'm trying to keep dependencies to a minimum.

While string manipulation sounds ugly / low level, we're not pulling data from a webpage or processing the HTML in any way. In fact we're done with it as soon as it's sent over the socket, anything we might need from the page, we already have in the form of class instances and variables.