Ok, so if I got you right, I should declare my instance variables only in the init function, right?
And class variables, specified in the body are available for all instances, even when changed.
Correct, that’s the essence. Anyhow, the other parts are important to work with them (and classes in general).
OK, but first you need to know some things about the python object system:
Classes are themselves objects, unlike in other languages (like Java, where they are just magical things brought to you by the JVM which don’t fit in Java in any other way). In fact, when you create a new object from a class, you are just calling it, like a function. You can create yourself objects that are callable, like in C++ with “operator ()”, and create in that way “Functor” objects (C++ programmers should have heard this, they are just that: Objects which are acting like a function, eg. you can call them). Functions are instances of the “Function” class (You can import it: “from types import FunctionsType” ).
>>> class A(object):
def __call__(self, a, b):
return a + b + 42
>>> a = A()
>>> a(3, 3)
48
Classes have so-called metaclasses, which are the classes of themselves. Since a class is somewhat of a “building plan” for a specific object, a metaclass is a building plan for the building plan. The standard metaclass every class is using is “type”. You can even create new metaclasses, either completly or you simply inherit from “type” and extend in the ways you need.
So, after that: A class variable of a class named “A” is a instance variable of the metaclass and has the specific property that it’s reachable from instances of the class “A”.
>>> class M(type):
def __init__(self, my_variable, *args, **kwargs):
self.my_variable = my_variable
type.__init__(self, *args, **kwargs)
>>> class A(object):
__metaclass__ = M
>>> A.my_variable
'A'
>>> A().my_variable
'A'
Don’t let this confuse you, it works just like other classes, with the exception that on creation of the class the class M is used to instantiate it - The class you have in the end is an instance of M. You can create standard classes even with type (or other metaclasses) yourself, by just instantiating it:
>>> def __init__(that_other_self, x):
that_other_self.x = x
>>> MyNewClass = type("Name", (object,), {"an_method": lambda self: self.x + 42, "__init__": __init__})
>>> my_new_class = MyNewClass(8)
>>> my_new_class
<__main__.Name object at 0xb7ea6f0c>
>>> my_new_class_instance = MyNewClass(8)
>>> my_new_class_instance
<__main__.Name object at 0xb7ea930c>
>>> my_new_class_instance.an_method()
50
The first parameter of a metaclass (constructor) is the name, the second the classes it should inherit from and the third a dictionary with all attributes (functions and variables) which an object of the class should own.
Since writing metaclasses everytime you need a class variable would be tiresome, python uses the class body for that:
>>> class A(object):
my_class_variable = 5
>>> A.my_class_variable
5
>>> A().my_class_variable
5
Note that therefore, every method you write is also a class variable - it is just reachable by the objects. When you change a class variable on the class, all objects notice:
>>> print a.my_class_variable, another_a.my_class_variable
5 5
>>> A.my_class_variable = 20
>>> print a.my_class_variable, another_a.my_class_variable
20 20
Now comes something very important: When you set a name in python on something, the most local namespace available is used. So when you change the variable on an instances, the instance namespace is used and you create a new instance variable, which shadows the old class variable:
>>> another_a.my_class_variable = 10
>>> print a.my_class_variable, another_a.my_class_variable
20 10
>>> print A.my_class_variable
20
So you create real instance variable in init, the constructor of the object:
>>> class A(object):
def __init__(self, some_name):
self.some_name = some_name
self.xyz = 42
>>> a = A(4)
>>> print a.some_name
4
>>> print a.xyz
42
>>> print A.some_name
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: type object 'A' has no attribute 'some_name'</module></stdin>
So, in short a class variable is something the class manages as opposed to something the object manages. When you change it on the class, all objects notice immediatly.
So when I construct something like you did in your code, this happens:
>>> class A(object):
handlers = []
def set_handler(self, handler):
self.handlers.append(handler)
>>> a, another_a = A(), A()
>>> a.set_handler("my_first_pseudo_handler")
>>> another_a.set_handler("another handler")
>>> print a.handlers
['my_first_pseudo_handler', 'another handler']
>>>
You see that in fact you have changed all handlers for all objects which exist. When you want handlers for every object, you have to create instance variables in init:
>>> class A(object):
... def __init__(self):
... self.handlers = []
... def set_handler(self, handler):
... self.handlers.append(handler)
...
>>> a, another_a = A(), A()
>>> a.set_handler("my_first_pseudo_handler")
>>> another_a.set_handler("another handler")
>>> print a.handlers
['my_first_pseudo_handler']
>>> print another_a.handlers
['another handler']
So it’s not right to use the class body for variables which you don’t need to be given in init.
One last important thing to notice is that this concept does not include methods of the metaclass, since functions you write in the class body are considered a variable and are just distributed to the instances of that class. To include this concept, python uses the class “classmethod”:
>>> class A(object):
def my_class_method(cls, bla):
print bla
print cls
my_class_method = classmethod(my_class_method)
>>> A.my_class_method(42)
42
<class '__main__.a'=""></class>
Note that you can also use classmethod as a decorator:
>>> class A(object):
@classmethod
def blub(cls):
pass
>>> A.blub
<bound method="" type.blub="" of="" <class="" '__main__.a'="">></bound>
(That is, by the way, really all decorators do - they expect to find an unary callable object and use it on the function (or class, in python greater than 2.6) you define next, rebinding the name. This was introduced since the concept is mighty (Functions as first order objects or you could also say the concept of “Higher order functions” ) and perfectly doable in python without decorators, but does hide the sense of the code when you don’t look closely what is written directly after the definition.)
For what do you need classmethods? There are many things you rather want the class to do than the object it creates (and it is really a great concept to work with), but the standard example are “alternate constructors”:
>>> class A(object):
def __init__(self, standard_format):
self.thing_in_standard_format = standard_format
@classmethod
def from_string(cls, another_format):
# So say we need in the normal constructor a number:
return cls(int(another_format))
>>> a = A.from_string("42")
>>> print a.thing_in_standard_format
42
>>> print type(a.thing_in_standard_format)
You could also write a classmethod named “from_file_object” which does just that. In other languages you would be forced to use normal functions which you group to the class, but that is really something the class should encapsulate, not it’s instances. Also note that this way you can inherit a class from another which uses classmethods and overwrite them: Any function which relies on the classmethod will not notice whether you gave it your subclass or the class from which you subclass inherited (this is called “polymorphism” ). Alone for this advantage it’s all worth it.
These concepts make python a really great language to work in. Many things in python are build upon a few basic concepts, and this hole class thing I just explained is one of them. All you have in python are objects from some classes, this is true everywhere, for every thing you can reach - things like “def some_function” are also just convenient abbreviations for the real thing, as I just showed with metaclasses. Python does much to hold up it’s minimalism, for example is type and instance of itself. This is done in C code and just works because of that.
Also, note that I inherited from “object” everywhere. The class system as I explained it was rewritten in python 2.1 and we have up to python 2.6 two sort of classes: The “Oldstyle” classes (which are the ones before 2.1) and the NewStyle Ones (which are those which inherit from object). In python 2.6 and greater you don’t need to inherit from object anymore and get NewStyle classes automatically. But before that, you should inherit from object. The OldStyle classes are deprecated and no longer supported (and are in many ways weaker than the NewStyle ones).
With these concepts in mind, you can do great things in python. The problem is that many tutorials don’t mention them. The official one I linked to does. Hope this helps you.
but I really had to know someone will somplain about my bad python coding style
I did not complain. I’m not even someone who has actually an use for the script. Although my primarely intention was to highlight improvements of the code, I have to admit that my answer was harsh, but not because I wanted to put you down or hurt you but because of… say, the way I got here. I know someone from this community since years very well and he showed me your post. I will not talk about why, but I am both worried about him as well as angry on him because of some things which got invoked by your post but are a issue for a while now. I want to apologize if I hurted you, but be assured it had nothing to do with “OH I CAN POWN SOME STRANGER ON THE INTERNETZ!1”-like behavior and I do not have anything against you personally. To clear this a bit up for you and in case you haven’t noticed already: I’m a longtime member of ]Imperium[.
So thanks to Str I won’t release anything else here, thank him if you think this is bad
Please reconsider your decision. Maybe someone will think of me again as arrogant, but this is a childish thing to do because of a post from which you do not know why it sounded the way it does, whilst, in the end, it is giving improvement suggestions.
Edit³: I have two sockets open for one reason: I don’t want to get my eventmode socket messed up by command output! And so everyone can do everything he likes with the other socket without affecting my event script, you understand? This is not intended for use of Players, it’s for Server Admins, which should know what they’ll do!
The code in question does not state anything of that. Also, just because you think someone experienced will use it does not free you from making it safe, maintainable and understandable, let alone making it free of trivial errors, like uninitialized variables. Especially copied code’s maintainability and understandability is the worst and makes grasping the main concepts harder.
Edit: Most mistakes come from my lack of time, so feel free to clean up my “crappy” code!
Well, the code does show deeper misunderstandings of python and/or programming in general, like using class variables and setHandler(), without clearing in any way whether it should be instance wide or class wide available. Please consider using the two links I gave you, since they will only clear things up.
Remember this is something created in Tai’s own free time. He atleast has the guts to make it opensource and gives others a peek in his code.
I do appreciate that. As I said, I got angry not because of tai. The code has had some influence, but the primarily reason is stated above.
Your code is utter bad.
from config import *
from FLEventBot import *
You don’t do this for nearly obvious reasons. A * import clutters you namespace full of names no one needs and breaks therefore encapsulation. More granularity of namespaces is in fact encouraged, and * exists only for convenience reasons in the interpreter or for short scripts, not for something to be unleashed unto other people.
print event['text']
A dictlike is not an event. You don’t mean what you say. Event indicates nothing about data or dictlikeness.
#FLEventdog(host,hookport,hookpass,DEBUG)
The standard is to use ## on outcommented sourcecode, but that is more or less you decision.
The Module:
Bad class name, what is that “dog”?
Absolutely no comments or documentation.
Why is the class inherited from threading.Thread? Does that bot plan to do anything other than get a bunch of callables and get busy? If not, you have that process, use it, don’t use threads.
You are using class variables so set up default arguments. A class variable expands over all instances, initialize instance variables in init(). Do you know what handlers will be doing when you create two “FLEventDogs”? It will fill them for all classes with the same handlers, and all instances will react. So either the use of a class variable is a design mistake or the method setHandler() is one, as it indicates setting a instance concrete handler. You use strings for identification of events. Because of the contextlessness of those strings besides the dict this is suboptimal. Instead you could have created a Event class which uses a tuple and outformulated names to denote events, eg:
class Event(object):
EVENT_TYPES = (FIRST, SECOND, THIRD) = tuple(range(3))
You encode line ending information directly into strings in form of \r\n. Heard of str.strip()? This would also be platform indepedent, since it is not possible to read from that code whether the \r\n is applicationspecific (bad design) or the normal windows line ending or has something to do with the connection. This is constantly redundant. Also, the need to parse strings to communicate with the program in question is the weakest possible communication strategy possible.
Your setuphook() method should contain a underscore on the beginning, since that is the official convention for API unstable and “private” methods. If anyone calls this method on your so called API the object may be left in an inconsistent state, depending on the concrete behavior of the application in case of:
1. Abrupt loss of connection since no shutdown was performed
2. New connection with same passwort whilst old connection is thought to be held
3. Your dependance of that concrete chathook instance. Since it is declared without an underscore, it is considered part of the API, which means
that you ensure that using it will not break anything, which is clearly wrong.
Your flow control is bad. You call a method which sets a flag in case of failure only to continue, confusing the reader of setuphook() (which is given no documentation also), and then to check on the next loop whether killed was set. Use return out of an loop and be not so confusing. Oh, I just see that I confused the method setuphook() with the method run(), since they are BOTH NEARLY COMPLETLY EQUAL. You copy and pasted code? You know that this is maybe the worst thing you could do as a programmer? Do you see what it causes? Confusion. So you setup hook, and do the same thing as in run(), whilst your KILLED IS NOT EVEN defined anywhere. That is what the constructor is for. And the statement I just wanted to say about the confusing flow control applies in run(), since you are doing it there.
In setuphook(), You also build a list named chars in which you insert text. Something is wrong with the last sentence, don’t you think? You will also loop in a while True loop endlessly should the protocol you attest FLHook break for some reason. Also, the \r\n story.
senduniverse() leaves the way directly open for insertion of valid XML code. And if it is really xml and FLHook just inserts “fmsgu” and the rest into some xml structure, which is clearly what it looks like, thats pretty bad and leaves the way for attacks directly open. If FLHook parses the string by it’s own by some sort of non-xml xml-like garbage, have fun with that build parser and a non standard, most likely not well defined pseudo-xml. You could also have used xml.etree.ElementTree, and the code would not much longer. It would also be better to read. Once again, you copy old code and doing the same thing as in setuohook().
Run() sets killed, which should be set be the threading.Thread.start() method, since that is the way Threads should be used. Again, you build a socket whilst leaving the other active, see above, again meaningless names and the same things copy and pasted again. Then, another block of copy pasted code, the same as the one I just read. stop() returns a useless bool and sets another one. Seriously, is self.killed used for anything other than flow control? Instance variable? A Thread should be alive from the moment it’s start() method returns and run() then terminates and that was it. No crude self.killed magic.
parseEvent() does not belong into that class. enumerate() exists. The parsing process is only valid if hook does clearly say that all events have bodys in a “a=b” form, without any characters. Odd, i thought it likes \r\n so much? Before is possible uninitialized and the flow control is messy because of before. data is too general. part is wrong named. if statements don’t need parenthesis. There are +=, -=, *= operators btw. The event is itself part of the data. WTF? And that “event” is in anyway a significant name is not given as a information anywhere else. See what will happen on a “event=cxf” “part”?
callHandlers(): Wrong identation. time.sleep()?
Your module doesnt contain a encoding information on the beginning of the file, a shebang would also be useful. Oh, and you ignore PEP8 completly and there is no standard notation in your code.
So, I’m hoping you are willing know to learn programming and python. Please start here:
http://docs.python.org/tutorial/index.html
http://python.org/dev/peps/pep-0008/