[Python] Packetbuffer

[topaz]

#packet buffer

__author__ = 'topaz'
__copyright = 'BSD License'

import string
import struct
import main

class pBuffer:

    def __init__(self):
        self.buffer = list()
        #declares self.buffer as a list
   
    def insertData(self, data):
        self.buffer.append(data)

    def insertInteger(self, data):
        if string.isdigit(data) == false:
            return
        else:
            data = struct.pack('I', data)
                #converts data into unsigned int and adds it into the buffer
                #converting integers into dwords/words is unnecessary, so we
                #simply pack it and append it
            self.buffer.append(data)

    def insertIntegerList(self, data):
        if string.isdigit(data) == false:
            return
        else:
            self.buffer.append('%s' %data)
            #adds a list of integers into the buffer

    def insertString(self, data):
        self.buffer.append(data)

    def insertNTString(self, data):
        self.buffer.append('%s.' % data)
        #adds string into buffer with a null terminator

    def makeInteger(self, data):
        data = struct.pack('I', data)

        return data
   
    def strToHex(self, data):
        data = data.encode('hex')

        return data
       
    def makeLong(self, data):
        data = struct.pack('L', data)
        #convert data into a Long and return it

        return data
   
    def hexToStr(self, data):
        data = data.decode('hex')
        #decodes data back into string and returns it

        return data

    def GetDWORD(self, data):
        #transform data into an integer and return it
        data = int(data)

        return data
       
    def GetWORD(self, data):
        data = int(data)

        return data

    def insertBYTE(self, data):
        data = Chr(data)
        #convert data into byte format and return it

        self.buffer.append(data)
   
    def sendPacket(self, packetID):
        main.transport.write(self.buffer)

        clear()

    def clear(self):
        #clears the buffer
        self.buffer[:] = []

If you find any errors, convention mistakes, or ways I could improve, post.

[Yegg]

IMO, there are way too many functions. As I suggested on AIM, just use insertData as your only function to add data to your buffer. It eliminates that mass of functions. Whether or not you may think having more functions for this makes things look more organized, I think that it just gives you more to look at and can maybe cause confusion, whereas having a single function do all the work you don't need to worry about such an issue.
insertData() should simply perform all tasks.

By the way, what's up with the:

Code Select Expand

self.buffer.append('%s' %data)
?
It would look much nicer if you did

Code Select Expand

self.buffer.append(data)

Under insertNTString() you have some redundance:

Code Select Expand

data = data & chr(0)
First off, this is Python not VB6 . & works differently in Python, use the + operator.

Also, that function should contain one line, so the code would look like:

Code Select Expand

def insertNTString(self, data):
    self.buffer.append('%s.' % data)
The above insertNTString() looks much nicer. chr(0) and a period are read the same way, so making a formatted string the way I did is the way to go.

Also, Python objects are case-sensitive, so Chr() would not work, but chr() would.

Under insertIntegerList() you have another error.

Code Select Expand

if string.isdigit(data) = false:
You used just one equal sign. Use either == or is. Or you can do:

Code Select Expand

if not string.isdigit(data):

What should the data argument in GetWORD and GetDWORD take? Could you provide an example? I know of a much more efficient method to obtaining a WORD or DWORD. It shuold also be done in ONE function. I don't see why you use so many functions, less functions makes code easier to follow.

I hope you've noticed that makeInteger() and makeLong() have identical tasks?

Here is an example of where two functions can become one.

Code Select Expand

    def insertString(self, data):
        self.buffer.append(data)

    def insertNTString(self, data):
        data = data & chr(0)
        #adds string into buffer with a null terminator

        self.buffer.append(data)
Instead of having two functions, you can have the following:

Code Select Expand

def insertString(self, data):
    self.buffer.append(data)
And depending on whether or not you want to include a period at the end, include that when sending data to the function.

When I showed you my InsertData() function, it was designed to take in all data that is to be added to the buffer. Why do you still have it there, and written the same way if you seem to have a function for every specific type of value which IMO is useless? Just use InsertData() alone. Instead of your GetWORD and GetDWORD, write a RetrieveData(). Such a function for retrieving data is very trivial to create, if you like I can even help you on it.

[topaz]

By the way, what's up with the:

Code Select Expand

self.buffer.append('%s' %data)

?
It would look much nicer if you did

Code Select Expand

self.buffer.append(data)

I don't think you can append lists like that...

Under insertNTString() you have some redundance:

Code Select Expand

data = data & chr(0)
First off, this is Python not VB6 . & works differently in Python, use the + operator.

Whoops.

Also, Python objects are case-sensitive, so Chr() would not work, but chr() would.

I was under the impression it was Chr(), not chr(). Thanks.

Under insertIntegerList() you have another error.

Code Select Expand

if string.isdigit(data) = false:
You used just one equal sign. Use either == or is. Or you can do:

Code Select Expand

if not string.isdigit(data):

My bad =P

What should the data argument in GetWORD and GetDWORD take? Could you provide an example? I know of a much more efficient method to obtaining a WORD or DWORD. It shuold also be done in ONE function. I don't see why you use so many functions, less functions makes code easier to follow.

The functions all have their own use; insertData is too simplistic, not to mention the operations you might need to perform before inserting it.

I hope you've noticed that makeInteger() and makeLong() have identical tasks?

My bad, intended them to have different packing instructions.


Instead of judging my coding habits/nuances/preferences, focus on mistakes & errors I make.

Code has been revised to fix previous errors.

[Yegg]

Simplistic is what you should be aiming for. Would you rather have readers of your code more confused or more focused on what is going on in the code and what does what?

With my philosophy, an InsertData() and InsertDataList() would be fine, because I know the regular InsertData() will not automatically work with lists unless you code it accordingly.

It is unnecessary to import string. Instead of:

Code Select Expand

import string
...
string.isdigit(data)
You can do:

Code Select Expand

data.isdigit()
IMO, that looks much nicer.

Also, people should be judged on their coding habits, IMO. Otherwise, how would they learn better coding methods (assuming but not implying that mine is the "better way")?

You have yet to explain your GetWORD and GetDWORD to me.

" insertData is too simplistic, not to mention the operations you might need to perform before inserting it."

You don't seem to grasp the idea of how InsertData() should operate. Instead of performing operations within the function itself, perform them outside and send them to the InsertData() function. Show me an example of how that could be a bad method.

[topaz]

Simplistic is what you should be aiming for. Would you rather have readers of your code more confused or more focused on what is going on in the code and what does what?

Adding more functions for different operations wouldn't confuse the readers; it would simply be more lengthy. Not my problem.

It is unnecessary to import string. Instead of:

Code Select Expand

import string
...
string.isdigit(data)
You can do:

Code Select Expand

data.isdigit()
IMO, that looks much nicer.

Didn't know you could do that, thanks.

Also, people should be judged on their coding habits, IMO. Otherwise, how would they learn better coding methods (assuming but not implying that mine is the "better way")?

Habits vary from person to person, and since there isn't any real issue with my code other than readability and your 'philosophy' of simplicity, shut up about it. I'm not going to change my code because you think it should be less lengthy.

You have yet to explain your GetWORD and GetDWORD to me.

You specify a parameter and pass it to getDWORD, and it does the rest.

You don't seem to grasp the idea of how InsertData() should operate. Instead of performing operations within the function itself, perform them outside and send them to the InsertData() function. Show me an example of how that could be a bad method.

Not a bad method, just cluttering the code that is doing the operations and passing it.

[Yegg]

I need an example of what you will be sending to GetWORD and GetDWORD. Your code of

Code Select Expand

data = int(data)
apparently converts data to an integer, however I'm not quite sure you know what you're doing with that function. Depending on what you need it for, you may need to use unpack().

[topaz]

I need an example of what you will be sending to GetWORD and GetDWORD. Your code of

Code Select Expand

data = int(data)
apparently converts data to an integer, however I'm not quite sure you know what you're doing with that function. Depending on what you need it for, you may need to use unpack().

check = data[start:end]
pBuffer.getDWORD(check)

[Yegg]

I need an example of what you will be sending to GetWORD and GetDWORD. Your code of

Code Select Expand

data = int(data)
apparently converts data to an integer, however I'm not quite sure you know what you're doing with that function. Depending on what you need it for, you may need to use unpack().

check = data[start:end]
pBuffer.getDWORD(check)

I'm still a bit curious as to if this will even work correctly. Would you be able to show me what your data variable will be holding? Also, have you tested that procedure yet?

[topaz]

I need an example of what you will be sending to GetWORD and GetDWORD. Your code of

Code Select Expand

data = int(data)
apparently converts data to an integer, however I'm not quite sure you know what you're doing with that function. Depending on what you need it for, you may need to use unpack().

check = data[start:end]
pBuffer.getDWORD(check)

I'm still a bit curious as to if this will even work correctly. Would you be able to show me what your data variable will be holding? Also, have you tested that procedure yet?

Not yet, still reading documentation on Twisted.