• Welcome to Valhalla Legends Archive.
 

[Python] Packetbuffer

Started by topaz, May 23, 2006, 07:14 PM

Previous topic - Next topic

topaz

Quote#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.
RLY...?

Yegg

#1
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:
self.buffer.append('%s' %data)
?
It would look much nicer if you did
self.buffer.append(data)

Under insertNTString() you have some redundance:
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:
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.
if string.isdigit(data) = false:
You used just one equal sign. Use either == or is. Or you can do:
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.
    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:
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

#2
Quote from: Yegg on May 23, 2006, 07:56 PM
By the way, what's up with the:
self.buffer.append('%s' %data)

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

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

Quote from: Yegg on May 23, 2006, 07:56 PM
Under insertNTString() you have some redundance:
data = data & chr(0)
First off, this is Python not VB6 :). & works differently in Python, use the + operator.

Whoops.

Quote from: Yegg on May 23, 2006, 07:56 PM
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.

Quote from: Yegg on May 23, 2006, 07:56 PM
Under insertIntegerList() you have another error.
if string.isdigit(data) = false:
You used just one equal sign. Use either == or is. Or you can do:
if not string.isdigit(data):

My bad =P

Quote from: Yegg on May 23, 2006, 07:56 PM
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.

Quote from: Yegg on May 23, 2006, 07:56 PM
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.
RLY...?

Yegg

#3
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:
import string
...
string.isdigit(data)

You can do:
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

Quote from: Yegg on May 23, 2006, 08:39 PM
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.

Quote from: Yegg on May 23, 2006, 08:39 PM
It is unnecessary to import string. Instead of:
import string
...
string.isdigit(data)

You can do:
data.isdigit()
IMO, that looks much nicer.

Didn't know you could do that, thanks.

Quote from: Yegg on May 23, 2006, 08:39 PM
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.

Quote from: Yegg on May 23, 2006, 08:39 PM
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.

Quote from: Yegg on May 23, 2006, 08:39 PM
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.
RLY...?

Yegg

I need an example of what you will be sending to GetWORD and GetDWORD. Your code of
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

#6
Quote from: Yegg on May 24, 2006, 05:46 AM
I need an example of what you will be sending to GetWORD and GetDWORD. Your code of
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)
RLY...?

Yegg

Quote from: Topaz on May 24, 2006, 06:40 PM
Quote from: Yegg on May 24, 2006, 05:46 AM
I need an example of what you will be sending to GetWORD and GetDWORD. Your code of
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

Quote from: Yegg on May 24, 2006, 07:46 PM
Quote from: Topaz on May 24, 2006, 06:40 PM
Quote from: Yegg on May 24, 2006, 05:46 AM
I need an example of what you will be sending to GetWORD and GetDWORD. Your code of
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.
RLY...?