[Mailman-Developers] Code Duplicacy

[Barry Warsaw]

On May 24, 2016, at 11:48 PM, Harshit Bansal wrote:

>I was writing the new 'IStyle' interface and after completing some part of
>it I realised that the code has become a bit repetitive as all the
>styleable attributes which are present in src/interfaces/mailinglist.py are
>to be copied to src/interfaces/styles.py in IStyle interface. For example,
>
>advertised = Attribute(
>    """Advertise this mailing list when people ask for an overview of the
>available mailing lists.""")
>
>This piece of code is now present in both src/interfaces/mailinglist.py and
>src/interfaces/styles.py.

This is in some respects a quirk held over from Mailman 2.  The MailList class
is a mixin of several other classes which hold both settings and functionality
(methods).  During the port to Mailman 3, I essentially collapsed all of that
into a single MailingList class.

Styles are a separate thing bolted onto the side of that, which is why they're
implemented as applying to a mailing list only at creation time.  You're right
that a style has many of those attributes, but also keep in mind that a style
can have a *subset* of attributes, as you should be able to see in the
existing style classes.  Styles are meant to be composable, so for example,
you could define an "announce-only" style that only modifies some of the
related attributes, but allows others to be defined in other style classes.

MailingList instances OTOH contain all the style related attributes.

>Is there any way to correct it or is it fine?  One approach that I am able to
>think of is to write one seperate interface containning all the styleable
>attributes and implement that interface both in src/model/mailinglist.py and
>src/model/styles.py.

I'm not sure.  Given that styles can be partial collections of attributes,
maybe it doesn't make sense to name them all in the IStyle interface.  Leaving
them only in the IMailingList doesn't have much of a functional effect; in
this case the interface isn't much more than documentation and marker (it
declares the @implementor class as being of that interface so it's easier to
look up).

Cheers,
-Barry