Make socket an object attribute by default instead of a class attribute

[zinsserzh]

(As per owners request, I'll open this kind of discussion in a new issue.) In ServerConnection and DCCConection, socket is initialized as a class attribute, which later masked by an object attribute of the same name in connect method. I think it's better to initialize it as an object attribute since self.socket is not supposed to be shared among different instances. Set uninitialized attributes to None is the best practice if it is not shared among instances https://stackoverflow.com/a/31467178

I think socket should be initialized in __init__:

def __init__(self, reactor):
    super(ServerConnection, self).__init__(reactor)
    self.connected = False
    self.features = features.FeatureSet()
    self.socket = None

[jaraco]

I prefer the pattern of setting (immutable) defaults in the class and overriding in each instance.

[zinsserzh]

I'm sorry, but this doesn't make sense. Class attribute is not meant for storing defaults. When people look for public properties, they will look at the __init__ function and see what are initialized there.

If you insist on your preference, that's fine, but I don't see consistency here. connected also has immutable default, why do you initialize it for each instance? Same thing for passive peeraddress peerport of DCCConnection

[jaraco]

The main advantage storing the default value in the class and overriding it in the instance is that it simplifies the behavior of the __init__, sometimes omitting its definition entirely. It also means the default value only needs to be defined/set conceptually once. I can see how others might have different sensibilities about this code, and I wish to avoid tweaking the code to suit the sensibilities of each new contributor who comes through - hence following the prime maintainer's sensibilities where there isn't a strong benefit to making a change (and in this case I see the benefit as negative).

I don't see consistency here. Why do you initialize it for each instance?

Good point. It's probably that way because I haven't made a systematic survey of the code to ensure consistency. Consistency is often desirable, especially if I felt strongly that the pattern needs to be taught/replicated. I don't feel strongly about it, but I'd probably lean toward consistency than not. I'll take a look at the parameters you mentioned and see if they deserve the same treatment.

[jaraco]

I've in a11cb30a30d7cf5eade1c3b4d98a240b7292858 and c8b8bc8, I've attempted to make the behavior more consistent.

[jaraco]

Also, in feature/delegate-properties, I've attempted another refactor that attempts to build on this concept and allow the 'connected' and other properties to simply reflect the state indicated by the other attributes (such as self.socket), but it doesn't work. The reconnect test fails because of the 'send quit' on disconnect behavior, which requires the connection to be present. So I'm punting on that for now.