0

I am trying to learn WHEN NOT to use:

  • classes
  • member variables

HERE IS THE CODE


access_point_detection_classes.py

from scapy.all import *

class Handler :
    def __init__(self) :
        self.wap_list = []
        self.TempWAP = None



    def process_packet_capture(self, packet_capture) :
            for packet in packet_capture :
                if packet.haslayer(Dot11) :
                    if packet.subtype == 8 :
                        bssid = packet.addr3
                        if packet.haslayer(Dot11Elt) :
                            p = packet[Dot11Elt]
                            while isinstance(p, Dot11Elt) :
                                if p.ID == 0 :
                                    essid = p.info
                                if p.ID == 3 :
                                    channel = ord(p.info)
                                p = p.payload

                            self.TempWAP = WirelessAccessPoint(bssid, essid, channel)

                            if self.check_for_duplicates() == False:
                                self.wap_list.append(self.TempWAP)

    def check_for_duplicates(self) :
        for w in self.wap_list :
            if self.TempWAP.bssid == w.bssid :
                return True
        return False

class WirelessAccessPoint :
    def __init__(self, bssid, essid, channel) :
        self.bssid = bssid
        self.essid = essid
        self.channel = channel

access_point_detection.py

from scapy.all import *
from access_point_detection_classes_module import *

def main() :

    H = Handler()

    packet_capture = sniff(count = 50)

    H.process_packet_capture(packet_capture)

    for w in H.wap_list :
        print w.bssid
        print w.essid
        print w.channel
        print '++++++++++++++++++++++++'




    # enable_managed_mode('wlan0')

if __name__ == "__main__" :
    main()

HERE ARE MY THOUGHTS


I am mostly sure I know WHEN to use a class and member variables. Look at my second class WirelessAccessPoint. The perfect need for a class in my opinion.

  • I need multiple objects of this type
  • Easier management of the data that comes along with this type

That's great, but look at my first class Handler. To be honest, I mostly created it so I could practice OOP. It does come in handy when I need to access wap_list. However, I initialize it without passing parameters for the constructor. Is that defeating the purpose? It seems that I could create all the methods of this class with only the self parameter to them. For instance ( haha ) I could make packet_capture a member variable of Handler and have the method process_packet_capture() take the parameter self.

At this point I feel like if I do not set some ground rules for myself I will try to make everything a class and everything a member variable and I will never pass a parameter besides self ever again!

  • When the costs outweigh the benefits. – Robert Harvey Jul 26 '16 at 5:21
  • @Robert What are the 'costs' of using a class / member variable? – TT4M.C Jul 26 '16 at 5:25
  • Mutability, overall increase in complexity, increased storage requirements, increased need for testing, increased need for documentation, higher maintenance costs. – Robert Harvey Jul 26 '16 at 5:26
2

However, I initialize it without passing parameters for the constructor. Is that defeating the purpose?

No, absolutely not. Handler, maybe misnamed, but in a sense it represents a collection object, which is perfectly reasonable without constructor parameters. A collection object maintains some state for it's duration, and, many collections are created with an initially empty set.

However, what I don't necessarily like is the use of a member variable for the short-lived state of TempWAP. I think a local variable and some parameter passing would be better here, no?

Maybe the example doesn't make it clear that TempWAP is otherwise used in your more complete implementation, or, as another alternative, you might actually have two classes here that are being conflated (one shorter lived and one longer lived), but based on what I can see in the question, I'd go for local variable instead of member variable here.

The point is that the lifetime of the two member variables in Handler seems inconsistent. So, these member should be differentiated, either one using a local variable, or in another class (overkill, though perhaps). To reiterate, when all your member variables in the same class have the same useful lifetime, you have a better class than otherwise.

  • TempWAP is used throughout the for loop. However, it is completely replaced and assigned with each iteration of the loop. Also, no other method of the class uses it. While wap_list is used in several methods as well as being appended to ( not reassigned ). Is this the reasoning? – TT4M.C Jul 26 '16 at 4:04
  • Yes, I think that is a good analysis. – Erik Eidt Jul 26 '16 at 5:04
2

Don't use classes and or member variables when they are not needed. As soon as you need them, well, use them.

I'm serious.

2

A variable should always have the smallest scope and the shortest lifetime possible. Therefore a class member is preferable to a global variable, and a local variable or parameter is preferable to a class member.

In your example, TempWAP is only used inside the execution of process_packet_capture() and could therefore be changed to a local and passed as a parameter to check_for_duplicates(). As a member variable, it is available after the execution, but does not have any useful value. wap_list on the other hand is accessed after the execution, so this has to be a member.

But depending on how you use Handler you could decide to rewrite process_packet_capture() to a stand-alone function which returns wap_list. This would make it simpler to use in my opinion, but this honestly comes down to preference. Some like to follow an OO paradigm throughout, while I personally prefer side-effect free stand-alone functions when possible.

The packet_capture variable is only used as input to process_packet_capture(), so given the above principles I see no reason to turn it into a member.

I do see a slight risk with having wap_list as a member rather than a return value. It means you might mistakenly access wap_list before process_packet_capture() is called, and if you call process_packet_capture() multiple times, wap_list will contain the accumulated results of all calls. This might be what you intend, but if not, then having it as a return value rather than member avoids this risk of misuse. Alternatively, you could have a packet_capture as a argument to the constructor and call process_packet_capture(packet_capture) directly in the constructor. This way you still have wap_list as a member but avoid the risk of misuse.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.