Refactoring code. Is this pyside design ok?

[Rudi Hammad]

Hello,

I am refactoring my rigging API code, and I am trying to make my code a bit more clean.

from PySide2 import QtWidgets, QtCore
from abc import ABCMeta, abstractmethod
 
class Widget():
    __metaclass__ = ABCMeta
   
    @abstractmethod    
    def __call__(self, **kwargs):
        for key in kwargs:
            if key == "t":
                self.title()
            if key == "n":
                self.widgetName()
            if key == "c":
                self.color()
            if key == "tt":
                self.toolTip()
            if key == "p":
                self.parent()
               
    def title(self):
        print "this will give the widget a title"
       
    def widgetName(self):
        print "this will give the widget a name"
       
    def color(self):
        print "this will give the widget color"
       
    def toolTip(self):
        print "this will pop up a tooltip"
       
    def parent(self):
        print "this will give a parent to the widget"
         
 
class PushButton(Widget):
 
    def __call__(self, **kwargs):
        super(PushButton, self).__call__(**kwargs)
 
 
class ComboBox(Widget):
 
    def __call__(self, **kwargs):
        super(ComboBox, self).__call__(**kwargs)
 
# Run
pushBtn = PushButton()
pushBtn(t="push me", n="pushBtnA")
pushBtn(t="push me too", n="pushBtB", c=("rgb(100, 25, 75)"), p=None, tt="I am here to help")
 
 
cmbBox = ComboBox()
cmbBox(n="comboBoxA")

Basically I have created an abstract class thas is inherited by diferent type of widgets.What do you think about that design. I like the fact is that in one line I can set an entire widget, and that it works with multiple types of widgets.

But According to "clean code" by Robert C.Martin (uncle bob) I might be breaking 2 rules here:
. the __call__ in the base class is too long and have multiple if statements.
. methods should maximum 3 arguments, with that design I can add as many as I want.

what do you think? Does this code make any sense?

Thanks,

R

[Justin Israel]

To be honest, I am not a fan of this design. 

What purpose does it serve to make __call__() an abstract method that already has an implementation, when your subclass just implements it by calling that impl? And then all the actual methods that would normally be abstract, are not? The point of an abstract base class it to make sure a subclass provides the implementation of one or more methods. 

I also find it strange to set a widget state by calling it with a bunch of single character keyword args. I've not seen this kind of design before. 

If you really want to have a single call to set the widget state, what is wrong with having a single explicit setter function with properly named keyword args? It's getting close to looking like a constructor. You are almost at that point anyways, where you would just use something like set_attrs(**kw). The clean code thing about max args I think refers to positional/named args that become part of the signature. I don't know of any rule that says you can't have a lot of kwargs. 

Thanks,

R

--
You received this message because you are subscribed to the Google Groups "Python Programming for Autodesk Maya" group.
To unsubscribe from this group and stop receiving emails from it, send an email to python_inside_m...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/python_inside_maya/e44bf7ee-783f-4080-bde6-3a286a1f65f5%40googlegroups.com.

[Rudi Hammad]

I see. I am going to the corner and think about what I've done. I' ll have a second look at it and try again.
Thanks,I appreciate the feedback

[Alok Gandhi]

I was about to answer but then Justin covered all the points precisely as I wanted to.

For the number of arguments, please note that there is no defined rule as far as I know. It depends on the use case.

From the Zen of python - 

"Explicit is better than implicit"

"Simple is better than complex"

I think if a method/function needs a certain number of arguments, then it does need them. It is better to be explicit about it and has as many as is required but also try to stay away from getting complex if you can solve the design at the backend and get rid of some the arguments, then better.

Moreover, the Qt framework is built around inheritance. It already provides and encourages to override the methods by subclassing the widgets. You can use the mixins to intercept the functionality if you want to override some of the methods by multiple inheritances and the super magic.

Btw, here is a super good talk from Raymond Hettinger about super. A must watch as it clears up a lot of misunderstandings about using super (which is my opinion is not usually exploited its fullest potential)

Cheers!

On Mon, Aug 12, 2019 at 10:58 AM Rudi Hammad <rudih...@gmail.com> wrote:

I see. I am going to the corner and think about what I've done. I' ll have a second look at it and try again.
Thanks,I appreciate the feedback

--
You received this message because you are subscribed to the Google Groups "Python Programming for Autodesk Maya" group.
To unsubscribe from this group and stop receiving emails from it, send an email to python_inside_m...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/python_inside_maya/9a56546f-93b6-4960-a9bf-3cde380d3ed2%40googlegroups.com.

--

[Justin Israel]

On Mon, Aug 12, 2019 at 2:58 PM Rudi Hammad <rudih...@gmail.com> wrote:

I see. I am going to the corner and think about what I've done. I' ll have a second look at it and try again.
Thanks,I appreciate the feedback

I hope I didn't offend you, as it wasn't my intention. Was just being honest about the request for feedback :-)

 

--
You received this message because you are subscribed to the Google Groups "Python Programming for Autodesk Maya" group.
To unsubscribe from this group and stop receiving emails from it, send an email to python_inside_m...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/python_inside_maya/9a56546f-93b6-4960-a9bf-3cde380d3ed2%40googlegroups.com.

[Rudi Hammad]

Of course not! On the contrary, I really appreciate it.

[Rudi Hammad]

Hey Alok,
Yes, there is no rule about the amount of arguments. It just that according to programmer like uncle Bob, with more that 40 years of experience, if yo have more than 3 args maybe you are going something wrong. He says that 3 is already a lot, and 4 is the devil work ( unless really really justified). Also he says that a function should never have a book arguments because that would probably mean that the code is not respecting the single responsibility principle. But that's a complete different topic.

My goal here is to avoid having to wright multiple lines to set a widget. So instead of using .setColor() then .setTitle() then .setObjectName() etc... Have it all in on call that works with any type of widget.

So I am just thinking about a way to abstract it, or maybe create just a wrapper. I don't know yet what would be a clean approach

[Rudi Hammad]

Okey, so this is another design that I am testing. This time I took a functor aproach (or at least that is what I think I did).

So I came up with this. What do you think, am I overthinking it?

Cheers,

R

from PySide2 import QtWidgets, QtCore

class WidgetFunctor(object):
   
    def __call__(self, widget, **kwargs):
        self.widget = widget
        for key in kwargs:
            self.setTitle(kwargs[key]) if key == "title" else None
            self.setObjectName(kwargs[key]) if key == "objectName" else None
            self.setSize(kwargs[key][0], kwargs[key][1]) if key == "size" else None
               
        return self.widget
   
    def setTitle(self, title):
        self.widget.setText(title)
        print "setting widget title success!"
       
    def setObjectName(self, objectName):
        self.widget.setObjectName(objectName)
        print "setting objectName success!"
       
    def setSize(self, width, height):
        self.widget.resize(width, height)
        print "setting parent success!"
       
    # And so on with more methods...
 
   
# Using the functor      
widgetFunctor = WidgetFunctor()
buttonA = widgetFunctor(QtWidgets.QPushButton(), title="foo", objectName="buttonA_btn", size=(60, 20))
buttonB = widgetFunctor(QtWidgets.QPushButton(), title="luu", objectName="buttonB_btn"))
comboBoxA = widgetFunctor(QtWidgets.QComboBox(), objectName="comboA_btn", size=(40, 50))


# Equivalent without functor (this is what I want to avoid)
buttonA = QtWidgets.QPushButton()
buttonA.setText("foo")
buttonA.setObjectName("buttonA_btn")
buttonA.resize(60, 20)
buttonB = QtWidgets.QPushButton()
buttonB.setText("luu")
buttonB.setObjectName("buttonB_btn")
comboBoxA = QtWidgets.QComboBox()
comboBoxA.setObjectName("comboA_btn")
comboBoxA.resize(40, 50)

[Marcus Ottosson]

My goal here is to avoid having to wright multiple lines to set a widget.

What’s wrong with using multiple lines to initialise a widget?

Equivalent without functor (this is what I want to avoid)

I don’t have 40 years of experience, but frankly that example is a lot easier to read IMO. Have you considered subclassing?

buttonA = MyPushButton()

class MyPushButton(QtWidgets.QPushButton):
  def __init__(self, text, parent=None):
    super(MyPushButton, self).__init__(text, parent)
    self.setObjectName("myPushButton")
    self.resize(60, 20)

--
You received this message because you are subscribed to the Google Groups "Python Programming for Autodesk Maya" group.
To unsubscribe from this group and stop receiving emails from it, send an email to python_inside_m...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/python_inside_maya/898d0237-0aed-4d2c-adcf-c4783ac5fe24%40googlegroups.com.

[Rudi Hammad]

Hey Marcus,
Subclassing is how I had my previous code. I am just exploring to see if can come up with something that could create any type of widget using one unique function, but
I might be over thinking it. Does the design I posted make any sense or is it completely wrong for any reason I don't see?
I forgot to mention that the methods are supposed to be private. So basically, the idea was to create a function object to set up any type of widget.

R

[Alberto Sierra]

Hello!

I only find that approach useful if for instance, you want to store all the created widgets for future referencing, something like: (sorry, I don’t have a Computer close to me)

class WidgetCreator(object):
LOADED_WIDGETS =dict()

You call function should start here
...
LOADED_WIDGETS.setdefault(kwargs.get(“objectName”), widget)

....


I found it too complicated anyway. I think that subclassing is the best approach here. You’ll have more individual control on each widget, and implementing future feature for an specific widget will be easier.

Hope it helps!

Thanks

Sent from my iPhone

> --
> You received this message because you are subscribed to the Google Groups "Python Programming for Autodesk Maya" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to python_inside_m...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/python_inside_maya/96a7b5f6-9ab4-4028-93a3-c57fe1df6c2b%40googlegroups.com.

[Justin Israel]

On Sun, Aug 18, 2019, 5:08 AM Rudi Hammad <rudih...@gmail.com> wrote:

Hey Marcus,
Subclassing is how I had my previous code. I am just exploring to see if can come up with something that could create any type of widget using one unique function, but
I might be over thinking it. Does the design I posted make any sense or is it completely wrong for any reason I don't see?

I agree with Marcus that the original multi line approach is more clear. Also, if your goal is just to reduce the number of lines, aren't you going to end up having to format the single functor call across multiple lines anyways to avoid a super long line as the parameters increase? 

The only benefit I can think of is that it does make it easier for you to store and reuse widget initialisation contexts as dictionaries so that you could stamp out different widget instances like:

    widgetA = factory(**typeA) 

But I don't know if that was your intended goal? You only refer to reducing line count. 

It's also easier to make a typo when you switch from calling first class setters to custom named parameters where it would skip any unrecognised params. Is that a consideration? 

I forgot to mention that the methods are supposed to be private. So basically, the idea was to create a function object to set up any type of widget.

The implementation of the functor actually doesn't need to store any state at all. Technically those methods could be static and just get passed a widget. It's also probably not a good idea to have it store the widget anyways since it prevents garbage collection or could hold onto something that gets deleted in C++. 

Also it seems like your impl calls every method multiple times, since it's doing it in a loop for the provided parameters. 

R

--
You received this message because you are subscribed to the Google Groups "Python Programming for Autodesk Maya" group.
To unsubscribe from this group and stop receiving emails from it, send an email to python_inside_m...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/python_inside_maya/96a7b5f6-9ab4-4028-93a3-c57fe1df6c2b%40googlegroups.com.

[Rudi Hammad]

Subclassing it is then!
It wasn't just about reducing the number of lines, but about having a layer on top that helped me setup real quick multiple uis. I didn't considered yet handling errors yet.I do have constant modules, so that would define different types of kwargs to apply.
Yes, I think it is getting too complicated...
It was an interesting exercise to do though.
Thanks all for the feedback.