** Changed in: horizon
Importance: Undecided => Wishlist
** Changed in: horizon
Status: In Progress => Fix Released
--
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Dashboard (Horizon).
https://bugs.launchpad.net/bugs/1182931
Title:
The use of class variables
Status in OpenStack Dashboard (Horizon):
Fix Released
Bug description:
I would like to ask is there is any reason for using so much class
variables in Horizon. Is there? I am quite new to python (ruby was my
world before) so I don't really know. And there may be something
inside Horizon that I don't see.
Using class attributes as default values for instance attributes is
considered to be an anti-pattern (in any language). As I checked
google and it seems this also applies for python community. Actually
it is anti-pattern to use class variables in most cases, because it is
not thread save and it can break the interface of objects.
I am trying to show how it could look like it it were encapsulated in
object, rather then working with class attributes.
So this how it looks now
example 1.
http://pastebin.com/kxas6AsD
and it could be rewritten to something like this
example 2.
http://pastebin.com/LHFuXWMb
Now what are the exact advantages of this approach:
1.
Example 2 is much more clear and concise, because now I see by first look
exactly what attributes are defined in each level of inheritance tree.
So instead of redefining a class variable in my concrete class of some action
I rather define the initializer and set the new defaults there.
2.
It will be consistent way how to set the defaults and the values of the
attributes. Now each attribute is set in slightly different manner and I have
to always look through a lot of code to see what value is actually getting
assigned. This tends to also very buggy.
3.
Relying on the python delegation (it first look to object, then superclass,
then superclass...) when I set the defaults can be quite confusing.
In example 1, although the value of important_attribute is set in
FilterAction(), it actually takes the class attribute from
MyGreatFilterAction(FilterAction). Because it takes the class attribute
important_attribute, that is closest to the object (so immediate super class) .
If we put much more classes to the inheritance tree, this gets more and more
confusing.
4.
All is encapsulated to the object, so it doesn't break any pattern and it is
thread safe. Now I can for example set the default value that will be different
for each user role and it still will be consistent (or something like that),
I really appreciate your work guys. I would be very glad if you could spent
some time on this and give me some feedback, and say if you like the idea. And
if you will like it then we can make it even nicer. :-)
I would gladly make these changes if it will be confirmed. Because I
know it won't be exactly easy.
Thank you very much for your help and time.
To manage notifications about this bug go to:
https://bugs.launchpad.net/horizon/+bug/1182931/+subscriptions
--
Mailing list: https://launchpad.net/~yahoo-eng-team
Post to : [email protected]
Unsubscribe : https://launchpad.net/~yahoo-eng-team
More help : https://help.launchpad.net/ListHelp
