Inefficient Code Assistance

This is the place for queries that don't fit in any of the other categories.

Inefficient Code Assistance

Postby KHarvey » Tue Mar 26, 2013 4:38 pm

I am attempting to pull what MAC address is assigned to each port on a Cisco switch using SNMP. I think it works, but I haven't had a chance to verify its accuracy as of yet.

When using SNMP to map the MAC address to a switch port you have to go through multiple OID's and compare them.
So here is the order and examples that you have to pull the OID's from the switch:
Code: Select all
MAC Address:
  .1.3.6.1.2.1.17.4.3.1.1.0.7.77.58.103.152 = Hex-STRING: 00 01 E6 3C 75 19
Bridge Port:
  .1.3.6.1.2.1.17.4.3.1.2.0.7.77.58.103.152 = INTEGER: 3
ifIndex:
  .1.3.6.1.2.1.17.1.4.1.2.3 = INTEGER: 10103
Port Name:
  .1.3.6.1.2.1.31.1.1.1.1.10103 = STRING: "Gi0/3"

The last 6 of the MAC OID matches the last six of the Bridge Port:
.1.3.6.1.2.1.17.4.3.1.1.0.7.77.58.103.152 = Hex-STRING: 00 01 E6 3C 75 19 | .1.3.6.1.2.1.17.4.3.1.2.0.7.77.58.103.152 = INTEGER: 3
The Integer of the Bridge Port matches the last OID of the ifIndex
.1.3.6.1.2.1.17.4.3.1.2.0.7.77.58.103.152 = INTEGER: 3 | .1.3.6.1.2.1.17.1.4.1.2.3 = INTEGER: 10103
The Integer of the ifIndex matches the last OID of the Port Name
.1.3.6.1.2.1.17.1.4.1.2.3 = INTEGER: 10103 | .1.3.6.1.2.1.31.1.1.1.1.10103 = STRING: "Gi0/3"

So in this example the MAC Address 00 01 E6 3C 75 19 is connected to Port Gi0/3

My code pretty much uses lists and dictionaries, and then I check to see if variables are in a list or a dictionary. I loop through all the of switches on my network (lpSQLIP[1]) and all the VLAN's (lpVLAN)on the switch. I am using snmpwalk pulls all of the possible OID's and variables for the MAC Address, Bridge Port, ifIndex, and Port Name.
Code: Select all
#Define Constants
strRunningScript = sys.argv[0]
intPartTotal = 2
intCurrent = 1
arrVLAN = []
arrOIDMAC = {}
arrBPMAC = {}
arrIfIndexMAC = {}
arrSwitchPortMAC = []

#Pull a list of all MAC Addresses in all VLANs
#.1.3.6.1.2.1.17.4.3.1.1.0.7.77.58.103.152 = Hex-STRING: 00 01 E6 3C 75 19
objMAC = subprocess.Popen("snmpwalk -v 2c -c %s@%s %s \
.1.3.6.1.2.1.17.4.3.1.1" %(strSNMPCommunity, lpVLAN, lpSQLIP[1]),
stdin=None, stdout=-1, stderr=None, shell=True)
while 1:
   strMACLine = objMAC.stdout.readline()
   if not strMACLine: break
   #Verify a MAC address exists
   if strMACLine.find("Hex-STRING:") != -1:
      #Assign the OID (key) and the MAC (variable) to a dictionary
      #The last 6 of the OID match the last 6 of the BridgePort OID
      arrOIDMAC.update({".".join(strMACLine.split("= Hex-STRING:")[0].split(".")[-6:]).strip():
         strMACLine.split("= Hex-STRING:")[1].strip().replace(" ", "-")})
#Pull a list of the Bridge Ports in all VLANs
#.1.3.6.1.2.1.17.4.3.1.2.0.7.77.58.103.152 = INTEGER: 3
objBP = subprocess.Popen("snmpwalk -v 2c -c %s@%s %s \
.1.3.6.1.2.1.17.4.3.1.2" %(strSNMPCommunity, lpVLAN, lpSQLIP[1]),
stdin=None, stdout=-1, stderr=None, shell=True)
while 1:
   strBPLine = objBP.stdout.readline()
   if not strBPLine: break
   #Verify a Bridge Port exists
   if strBPLine.find("= INTEGER:") != -1:
      #Verify the BP is a key in the arrOIDMAC dictionary
      if ".".join(strBPLine.split("= INTEGER:")[0].split(".")[-6:]).strip() in arrOIDMAC:
         #Assign the BP and MAC to a dictionary
         #BP maps to ifIndex OID
         arrBPMAC.update({strBPLine.split("= INTEGER:")[1].strip():
            arrOIDMAC[".".join(strBPLine.split("= INTEGER:")[0].split(".")[-6:]).strip()]})
#Pull a list of the ifIndexes in all VLANs
#.1.3.6.1.2.1.17.1.4.1.2.3 = INTEGER: 10103
objIfIndex = subprocess.Popen("snmpwalk -v 2c -c %s@%s %s \
.1.3.6.1.2.1.17.1.4.1.2" %(strSNMPCommunity, lpVLAN, lpSQLIP[1]),
stdin=None, stdout=-1, stderr=None, shell=True)
while 1:
   strIfIndexLine = objIfIndex.stdout.readline()
   if not strIfIndexLine: break
   #Verify a ifIndex exists
   if strIfIndexLine.find("= INTEGER:") != -1:
      #Verify the IfIndex is a key in the arrBPMAC dictionary
      if ".".join(strIfIndexLine.split("= INTEGER:")[0].split(".")[-1:]).strip() in arrBPMAC:
         #Assign the ifIndex name and the MAC to a dictionary
         #ifIndex maps to Port OID
         arrIfIndexMAC.update({strIfIndexLine.split(" = INTEGER:")[1].strip():
            arrBPMAC[".".join(strIfIndexLine.split("= INTEGER:")[0].split(".")[-1:]).strip()]})
#Pull a list of Port names in all VLANs
#.1.3.6.1.2.1.31.1.1.1.1.10103 = STRING: "Gi0/3"
objPN = subprocess.Popen("snmpwalk -v 2c -c %s@%s %s \
.1.3.6.1.2.1.31.1.1.1.1" %(strSNMPCommunity, lpVLAN, lpSQLIP[1]),
stdin=None, stdout=-1, stderr=None, shell=True)
while 1:
   strPNLine = objPN.stdout.readline()
   if not strPNLine: break
   #Verify a ifIndex exists
   if strPNLine.find("= STRING:") != -1:
      #Verify the Port Name is a key in the arrIfIndexMAC
      if ".".join(strPNLine.split("= STRING:")[0].split(".")[-1:]).strip() in arrIfIndexMAC:
         arrSwitchPortMAC.append((lpSQLIP[0],
         strPNLine.split("= STRING:")[1].strip(),
         arrIfIndexMAC[".".join(strPNLine.split("= STRING:")[0].split(".")[-1:]).strip()]))
#Clear out dictionaries so that info is not carried into other VLANs
arrOIDMAC = {}
arrBPMAC = {}
arrIfIndexMAC = {}

#The set fuction removes all duplicates from a list,
#and the list function turns the set string back into a list
arrSwitchPortMAC = list(set(arrSwitchPortMAC))

I still have a bit of code clean up to do, but this is pretty much how I am doing it and how I have been doing it.

For some reason this code looks really ugly to me. I think there is a better way of doing this, but with my limited knowledge I am unsure what that better way would be.

I have thought about creating a function where I submit the OID to walk, VLAN, IP Address, OID to parse (last 6 or last 1 in most cases), and the type of variable (STRING, Hex, INTEGER), and have that return the list or dictionary.
But even then I am not sure that would be a good way of handling it.

So my question is, how bad is my code and do you have any tips or suggestions for it?
KHarvey
 
Posts: 34
Joined: Tue Mar 19, 2013 5:13 pm
Location: US

Re: Inefficient Code Assistance

Postby setrofim » Tue Mar 26, 2013 5:15 pm

KHarvey wrote:So my question is, how bad is my code

It's pretty bad (hey, you asked); not the worst I've seen though.

KHarvey wrote: and do you have any tips or suggestions for it?

Sure (in no particular order):
  • As you have observed, you have a bunch of repeated code that begs to be factored into a function (or possibly several functions).
  • The fact you're using a lot of global state, suggests that you might want to use a class with methods.
  • Use blank lines to separate logical sections of code -- it makes it more readable.
  • Lines like this:
    Code: Select all
    arrOIDMAC.update({".".join(strMACLine.split("= Hex-STRING:")[0].split(".")[-6:]).strip():

    Are very difficult to understand and will leave you scratching your head when you come back to the code in a few moths' time. Break these up into several statements, assigning the intermediate result to a meaningfully-named variable. (also, in that particular line, you have a stray colon at the end).
  • Do not use Hungarian notation (do not identify the type of the variable in its name). This practice is generally considered outdated, and it doesn't really make sense in Python (since Python variables do not inherently have a type associated with them, and you shouldn't really care what type there are anyway). Hungarian notation does not add any useful information and makes you code uglier and harder to read.
  • Rather than using camel case, python variables and functions a typically all lower case with underscores, e.g 'running_script' rather than 'runningScript'. Classes should be title-cased, e.g. MyClassName.
  • Use all upper case and underscores for constants (variables that are intended to have the the same value throughout the life time of your program).
  • Use "while True:" rather than "while 1:"; this ain't C.
  • If you're going to break a statement across multiple lines (and you should avoid doing that -- keep your statements short), indent the continuation. i.e. rather than
    Code: Select all
    objPN = subprocess.Popen("snmpwalk -v 2c -c %s@%s %s \
    .1.3.6.1.2.1.31.1.1.1.1" %(strSNMPCommunity, lpVLAN, lpSQLIP[1]),
    stdin=None, stdout=-1, stderr=None, shell=True)

    do
    Code: Select all
    objPN = subprocess.Popen("snmpwalk -v 2c -c %s@%s %s \
                        .1.3.6.1.2.1.31.1.1.1.1" %(strSNMPCommunity, lpVLAN, lpSQLIP[1]),
                        stdin=None, stdout=-1, stderr=None, shell=True)

    or event better:
    Code: Select all
    # Specify this once
    COMMAND_TEMPLATE = "snmpwalk -v 2c -c %s@%s %s .1.3.6.1.2.1.31.1.1.1.1"

    command = COMMAND_TEMPLATE % (snmp_community, vlan, sqlip[1])
    pn = subprocess.Popen(command, stdin=None, stdout=-1, stderr=None, shell=True)
  • Avoid using list indexes too much. Are you going to remember what strMACLine.split("= Hex-STRING:")[1] is? Assign these to variables with meaningful names.

The preferred coding conventions for Python are laid out in PEP8. Sticking to that will go a long way to ensuring that your code is readable and maintainable.
setrofim
 
Posts: 288
Joined: Mon Mar 04, 2013 7:52 pm

Re: Inefficient Code Assistance

Postby KHarvey » Tue Mar 26, 2013 6:21 pm

setrofim wrote:
KHarvey wrote:So my question is, how bad is my code

It's pretty bad (hey, you asked); not the worst I've seen though.

That's what I was afraid of, but I didn't actually think it was my coding standard that was bad, just thought it was my actual code :oops:

setrofim wrote:The fact you're using a lot of global state, suggests that you might want to use a class with methods.

I don't understand what a global state is. Honestly I haven't built many classes, most of my code are loops and functions.

setrofim wrote:Use blank lines to separate logical sections of code -- it makes ti more readable.

Sorry about that, I hadn't finished my code clean up yet, so I am missing some indentations and spacing along with some comments.

setrofim wrote:[*]Lines like this:
Code: Select all
arrOIDMAC.update({".".join(strMACLine.split("= Hex-STRING:")[0].split(".")[-6:]).strip():

Are very difficult to understand will leave you scratching your head when you come back to the code in a few moths' time. Break these up into several statements, assigning the intermediate result to a meaningfully-named variable. (also, in that particular line, you have a stray colon at the end).

Yeah I have always been torn about how complicated that code should be. I have actually come back to this code several times, and sometimes it takes me a minute to remember, but I do so much parsing in my code that it has become second nature. Coming from a slight Java background I developed a habit of clearing variables after I am done with them. Sorry but after dealing with so many Java apps that had never ending memory leaks because Garbage Collection never picked them up, I started to run del() or unset() on all my variables once I was done with them. I still do this in Python and PHP. This has added some complexity to my coding habits as when I create the variable I have to destroy the variable as well.

setrofim wrote:Do not use Hungarian notation (do not identify the type of the variable in its name). This practice is generally considered outdated, and it doesn't really make sense in Python (since Python variables do not inherently have a type associated with them, and you shouldn't really care what type there are anyway). Hungarian notation does not add any useful information and makes you code uglier and harder to read.

I never knew that there was a name for this. It's something that I have been doing for many years and through many different languages.
This may or may not be a habit that I am able to break. Reading online about this, one of the main reasons to not do this is that IDE's handle all of this info for you. Most of my coding is done in simple text editors that don't have any typing built in. Honestly I am unsure how I would code without these.
Now that I think about this I think it was my C instructor and later a DBA that beat this into me.

setrofim wrote:Rather than using camel case, python variables and functions a typically all lower case with underscores, e.g 'running_script' rather than 'runningScript'. Classes should be title-cased, e.g. MyClassName.

LOL! Once again I didn't know there was a name for this. I mainly used camel case for speed of coding. I can type way faster without using underscores. Although most code that I read now uses underscores, so that could be something that I could easily change.

setrofim wrote:Use all upper case and underscores for constants (variables that are intended to have the the same value throughout the life time of your program).
Use "while True:" rather than "while 1:"; this ain't C.

Piece of cake, those are simple changes.

setrofim wrote:If you're going to break a statement across multiple lines (and you should avoid doing that -- keep your statements short), indent the continuation. i.e. rather than
Code: Select all
objPN = subprocess.Popen("snmpwalk -v 2c -c %s@%s %s \
.1.3.6.1.2.1.31.1.1.1.1" %(strSNMPCommunity, lpVLAN, lpSQLIP[1]),
stdin=None, stdout=-1, stderr=None, shell=True)

do
Code: Select all
objPN = subprocess.Popen("snmpwalk -v 2c -c %s@%s %s \
                    .1.3.6.1.2.1.31.1.1.1.1" %(strSNMPCommunity, lpVLAN, lpSQLIP[1]),
                    stdin=None, stdout=-1, stderr=None, shell=True)

or event better:
Code: Select all
# Specify this once
COMMAND_TEMPLATE = "snmpwalk -v 2c -c %s@%s %s .1.3.6.1.2.1.31.1.1.1.1"

command = COMMAND_TEMPLATE % (snmp_community, vlan, sqlip[1])
pn = subprocess.Popen(command, stdin=None, stdout=-1, stderr=None, shell=True)


Honestly up until a couple of months ago I didn't break my lines apart. But someone else looking at my code said that it was best practice never to go above char 80 and to break my lines up to make them more readable. That way you don't have the horizontal scroll.
As I said earlier I hadn't finished my code cleanup completely yet, that's why I was missing my indentations.
Also I didn't know that I could assign the %s parameter to another variable and still call the parameter later on. That will help me to greatly simplify my code. Although, at that point if I am using it that much then I should probably use a function instead.

setrofim wrote:Avoid using list indexes too much. Are you going to remember what strMACLine.split("= Hex-STRING:")[1] is? Assign these to variables with meaningful names.

I see your point with this. But if I am looping through a list, I would need to continually create and destroy the variable to make sure that the last variable in the loop isn't carried over to the first variable in the loop. I know I have an example of this somewhere, but I can't remember where. It was a piece of code that I had to check the Type before I assigned the variable as I kept getting NoneType errors. So in the case of the NoneType it would take the last variable that was assigned to it. Most likely this was caused by my poor coding at the time, but I couldn't figure out a better way of handling it.
Actually I think it was a question I posed on here a while ago.

setrofim wrote:The preferred coding conventions for Python are laid out in PEP8. Sticking to that will go a long way to ensuring that your code is readable and maintainable.

I will go ahead and read through those and see if I can incorporate some of my eccentricities into a proper coding method.

Thanks for the tips.

As far as the code goes, is that the proper way to do the comparisons?
Where I assign a variable to the key and then do an "in" to see if the key exists and then create another list/dictionary once again based off the variable from the key in the previous list / dictionary?
KHarvey
 
Posts: 34
Joined: Tue Mar 19, 2013 5:13 pm
Location: US

Re: Inefficient Code Assistance

Postby setrofim » Tue Mar 26, 2013 8:22 pm

KHarvey wrote:I don't understand what a global state is.

Global variables. You should avoid using them as much as possible (global constants are OK).

KHarvey wrote:Sorry about that, I hadn't finished my code clean up yet,

Always clean up your code before posting. As someone seeking help, it is in your interest to make your code as easy to understand as possible. A lot of people won't even bother reading when they see a lot of messy code. Also, we won't waste our time (and yours) telling you things you already know.

KHarvey wrote:so I am missing some indentations and spacing along with some comments.

More comments != better. Comments like this:
Code: Select all
    #Verify a MAC address exists
    if strMACLine.find("Hex-STRING:") != -1:

Just clutter up the code and make it harder to understand. Ideally, code should be self-documenting. Python is expressive enough do let do that. E.g. in the above case
you can do
Code: Select all
def mac_exists(line):
    return 'Hex-STRING:' in line

# <snip>...

    if mac_exists(strMACLine):
        # ...

It's more verbose, sure, and in practice, I probably would refactor that code differently. But the point is that you should aim to write code that is clear enough without comments dotted all over the place. In a lot Python code, the only comments you will find are docstrings at the beginning of functions and classes (and you should definitely write those).

KHarvey wrote:Sorry but after dealing with so many Java apps that had never ending memory leaks because Garbage Collection never picked them up, I started to run del() or unset() on all my variables once I was done with them.

Garbage collection is not something you need to worry about in Python (unless you're writing very specialist applications). You don't need to explicitly del stuff either (you should definitely close() resources such as file handles and db connections though).

KHarvey wrote:This may or may not be a habit that I am able to break.
....
Reading online about this, one of the main reasons to not do this is that IDE's handle all of this info for you.

It's the most stated reason. IDE's don't really have anything to do with it though. Especially in Python. In Python variables don't have types (objects that get dynamically assigned to variables do), so putting type information into a variable's name is meaningless, and worse, could be misleading. Breaking the habit will take effort, but it's worth doing.

KHarvey wrote: Most of my coding is done in simple text editors that don't have any typing built in. Honestly I am unsure how I would code without these.

It's a crutch to hide deeper issues with your code. Follow these two guidelines and you won't miss it:
  • Write shorter functions and avoid global variables -- if a variable is used close to where it is created, you won't need a reminder of what type it is.
  • Use meaningful variable names. A variable called start_date is obviously a date; a variable called client_name is obviously a string (or unicode if that's what you're working with), something_index is obviously an integer. With names like these, a type prefix is redundant.

KHarvey wrote:Now that I think about this I think it was my C instructor and later a DBA that beat this into me.

Unfortunately, there was a time when Hungarian notation was common and was even considered good practice (IIRC, Code Complete recommended it). It is now more or less consensus within the industry that Hungarian notation is more trouble than it's worth.

KHarvey wrote:I mainly used camel case for speed of coding. I can type way faster without using underscores.

Your code will be read a lot more than it is written (by others as well as yourself). Therefore you should put more effort into writing code if it means less effort when reading it (especially if its code you intend others to read).

KHarvey wrote:Honestly up until a couple of months ago I didn't break my lines apart. But someone else looking at my code said that it was best practice never to go above char 80 and to break my lines up to make them more readable. That way you don't have the horizontal scroll.

The 80 columns limit is an accident of history (this came from VT100, a popular terminal way back in the day, that had an 80 column wide screen). The important thing is to keep your lines short, rather than stick to a particular limit (though 80 columns is as good a guide line as any).

KHarvey wrote:
setrofim wrote:Avoid using list indexes too much. Are you going to remember what strMACLine.split("= Hex-STRING:")[1] is? Assign these to variables with meaningful names.

I see your point with this. But if I am looping through a list, I would need to continually create and destroy the variable to make sure that the last variable in the loop isn't carried over to the first variable in the loop.

If you're looping though a list, you shouldn't be using indexes at all:
Code: Select all
for element in my_list:
    # do stuff with element...

No need to worry about incrementing indexes, assigning variables to the right index etc.

KHarvey wrote:It was a piece of code that I had to check the Type before I assigned the variable

In general, you shouldn't be doing that either. Most of the time, you shouldn't care what the type an object is (again, variables don't have types) -- as long as it has the right attributes to work in the context (this is the duck typing principle).


KHarvey wrote:
setrofim wrote:The preferred coding conventions for Python are laid out in PEP8. Sticking to that will go a long way to ensuring that your code is readable and maintainable.

I will go ahead and read through those and see if I can incorporate some of my eccentricities into a proper coding method.

Unlike with other languages (I can think of go as another exception), Python has a single coding style that has been (almost) universally adopted and you need a good reason not to use it. If you don't follow PEP8, the Python interpreter won't complain, but other people reading your code will. So if you're writing code intended to be read by others, PEP8 isn't really optional. You don't have to follow it religiously (e.g. the 80 column rule often gets "softened") but the more you deviate from it, the less readable your code would be to other Python developers. If you're just writing code for yourself, it's less critical (but of course then you'd just be developing more bad habits). PEP8 is not just an arbitrary set of purely stylistic guidelines (well, OK, some of it is stylistic), it results in more readable, more maintainable and less bug prone code. Again, code is read a lot more than it is written (and debugging code is about twice as hard as writing it). So if you're choosing convenience while writing over writing cleaner code, you're just setting yourself up for trouble further down the line.

Above all, forget the stuff you've learnt with C and Java. Python is a very different language. Too often I've seen people writing what is essentially C code in Python. It results in truly horrible, hard to understand code. It's like C++ first appeared, there were a bunch of C coders that were basically wrapping their C code in one massive class and then were complaining that C++ was slower and didn't give them any benefit over C. Learn the language. Go through the tutorial, even if you're already experienced in other languages.

KHarvey wrote:As far as the code goes, is that the proper way to do the comparisons?
Where I assign a variable to the key and then do an "in" to see if the key exists and then create another list/dictionary once again based off the variable from the key in the previous list / dictionary?

You shouldn't be using indexes all over the place, and explicitly checking if the result of find() is -1 is unpythonic; apart from that looks more or less right. Your code has bigger issues at the moment. You should clean it up before asking about it's structure.
setrofim
 
Posts: 288
Joined: Mon Mar 04, 2013 7:52 pm

Re: Inefficient Code Assistance

Postby KHarvey » Wed Mar 27, 2013 9:59 pm

setrofim wrote:
KHarvey wrote:I don't understand what a global state is.

Global variables. You should avoid using them as much as possible (global constants are OK).

Yeah, I know that global variables are bad. Although I am not at a level yet where I know how to avoid them completely. Most of the things that I build are modular, and the variables only exist in one module, but are normally global in the module.

setrofim wrote:
KHarvey wrote:so I am missing some indentations and spacing along with some comments.

More comments != better. Comments like this:
Code: Select all
    #Verify a MAC address exists
    if strMACLine.find("Hex-STRING:") != -1:

Just clutter up the code and make it harder to understand. Ideally, code should be self-documenting. Python is expressive enough do let do that. E.g. in the above case
you can do
Code: Select all
def mac_exists(line):
    return 'Hex-STRING:' in line

# <snip>...

    if mac_exists(strMACLine):
        # ...

It's more verbose, sure, and in practice, I probably would refactor that code differently. But the point is that you should aim to write code that is clear enough without comments dotted all over the place. In a lot Python code, the only comments you will find are docstrings at the beginning of functions and classes (and you should definitely write those).

I have been pulled in a couple of different directions on this. I have been in positions where they wanted the code to be self explanatory without comments (other than the description at the top), and I have had it where they wanted everything commented. I normally veer towards more comments rather than less, but I agree that the comment for "verify mac address exists" was a little much.

setrofim wrote:
KHarvey wrote:Sorry but after dealing with so many Java apps that had never ending memory leaks because Garbage Collection never picked them up, I started to run del() or unset() on all my variables once I was done with them.

Garbage collection is not something you need to worry about in Python (unless you're writing very specialist applications). You don't need to explicitly del stuff either (you should definitely close() resources such as file handles and db connections though).

Garbage collection in Python is something that I need to look at a little more closely. Mainly to make sure things don't stay resident in memory even after the Python script / process has completed. I will probably keep my habit of closing variables just in case I go back to another language that needs it.

setrofim wrote:
KHarvey wrote:This may or may not be a habit that I am able to break.
....
Reading online about this, one of the main reasons to not do this is that IDE's handle all of this info for you.

It's the most stated reason. IDE's don't really have anything to do with it though. Especially in Python. In Python variables don't have types (objects that get dynamically assigned to variables do), so putting type information into a variable's name is meaningless, and worse, could be misleading. Breaking the habit will take effort, but it's worth doing.

I now understand the difference between Apps Hungarian Notation and System Hungarian Notation. I still whole heartily agree with Hungarian Notation in certain circumstances (Apps). A good example that I read was using it to determine safe and unsafe variables. So something that has been cleaned for use in HTML or in SQL. Having a prefix to identify that a variable has been cleaned I believe is good practice as it can help to limit security threats. But using it for strings and integers is a little excessive. I may still use it to identify my lists and dictionaries, and my loop variables associated with them.
This is more of a coding preference that I will expound upon later in this post.

setrofim wrote:
KHarvey wrote: Most of my coding is done in simple text editors that don't have any typing built in. Honestly I am unsure how I would code without these.

It's a crutch to hide deeper issues with your code. Follow these two guidelines and you won't miss it:
  • Write shorter functions and avoid global variables -- if a variable is used close to where it is created, you won't need a reminder of what type it is.
  • Use meaningful variable names. A variable called start_date is obviously a date; a variable called client_name is obviously a string (or unicode if that's what you're working with), something_index is obviously an integer. With names like these, a type prefix is redundant.

To me this is just replacing my prefix with a suffix. I think that most of my variables contain the info needed, I was just adding a little more to them at the beginning rather than the ending.
I do see your point about writing shorter functions, and using my variables closer to where I use them. Although with my new coding endeavor I did find myself a little confused by my variables and got lost a couple of times trying to figure out which was which. I am not really sure that it was caused by the lack of Hungarian Notation, but more that I was creating a lot of interim variables that were virtually the same thing as other variables. I think that I need to work on that piece.

setrofim wrote:
KHarvey wrote:Now that I think about this I think it was my C instructor and later a DBA that beat this into me.

Unfortunately, there was a time when Hungarian notation was common and was even considered good practice (IIRC, Code Complete recommended it). It is now more or less consensus within the industry that Hungarian notation is more trouble than it's worth.

Reading through my Microsoft books it looks as though it is still good practice to do this, or rather all of their examples still do this. Although my books are a few years old still.
In the end, after Googling for several hours I never did see a consensus on whether Hungarian Notation was bad or good. It seemed fairly split. I will adjust my coding style to an extent but I don't think I will get rid of it all together.

setrofim wrote:
KHarvey wrote:I mainly used camel case for speed of coding. I can type way faster without using underscores.

Your code will be read a lot more than it is written (by others as well as yourself). Therefore you should put more effort into writing code if it means less effort when reading it (especially if its code you intend others to read).

Reading through the PEP it appears that camel case is still used. While in my new code I switched to underscores, somethings I may still use CamelCase on, but either way I will stay consistent. I never had an issue reading CamelCase or the underscores. The underscores to me are a little harder to type, and add an extra char(space) in the variables.

setrofim wrote:
KHarvey wrote:Honestly up until a couple of months ago I didn't break my lines apart. But someone else looking at my code said that it was best practice never to go above char 80 and to break my lines up to make them more readable. That way you don't have the horizontal scroll.

The 80 columns limit is an accident of history (this came from VT100, a popular terminal way back in the day, that had an 80 column wide screen). The important thing is to keep your lines short, rather than stick to a particular limit (though 80 columns is as good a guide line as any).

I have now added an extra indentation to my code for the lines that I have broken apart. I moved to a more PHP / Java stylized way of using my parens though, which some may complain about
Code: Select all
snmp_command = (
    "snmpwalk -v %s -c %s%s %s %s"
    %(version, strSNMPCommunity, vlan, ip, oid)
)

Granted the indentations should tell me where the block ends, but moving the parenthesis down to the next line at the same indentation allows me to keep a little cleaner lines all the way down.

setrofim wrote:
KHarvey wrote:It was a piece of code that I had to check the Type before I assigned the variable

In general, you shouldn't be doing that either. Most of the time, you shouldn't care what the type an object is (again, variables don't have types) -- as long as it has the right attributes to work in the context (this is the duck typing principle).

Yeah, it didn't feel right when I did it. But it was the only solution that I had found, and others had found at the time. I am now reviewing and fixing some of the bugs in my code, so most likely I will run across this piece of code again. When I do if I remember, or if I can't find a better way of doing it then I will post and ask for help again.
Pretty much the problem had to do with NoneTypes and it kept raising exceptions. The two options that I was given was to use a try except, or check for NoneType. But I will eventually find the piece of code that I did that in and figure out a better way of doing it.

setrofim wrote:
KHarvey wrote:
setrofim wrote:The preferred coding conventions for Python are laid out in PEP8. Sticking to that will go a long way to ensuring that your code is readable and maintainable.

I will go ahead and read through those and see if I can incorporate some of my eccentricities into a proper coding method.

Unlike with other languages (I can think of go as another exception), Python has a single coding style that has been (almost) universally adopted and you need a good reason not to use it. If you don't follow PEP8, the Python interpreter won't complain, but other people reading your code will. So if you're writing code intended to be read by others, PEP8 isn't really optional. You don't have to follow it religiously (e.g. the 80 column rule often gets "softened") but the more you deviate from it, the less readable your code would be to other Python developers. If you're just writing code for yourself, it's less critical (but of course then you'd just be developing more bad habits). PEP8 is not just an arbitrary set of purely stylistic guidelines (well, OK, some of it is stylistic), it results in more readable, more maintainable and less bug prone code. Again, code is read a lot more than it is written (and debugging code is about twice as hard as writing it). So if you're choosing convenience while writing over writing cleaner code, you're just setting yourself up for trouble further down the line.

Above all, forget the stuff you've learnt with C and Java. Python is a very different language. Too often I've seen people writing what is essentially C code in Python. It results in truly horrible, hard to understand code. It's like C++ first appeared, there were a bunch of C coders that were basically wrapping their C code in one massive class and then were complaining that C++ was slower and didn't give them any benefit over C. Learn the language. Go through the tutorial, even if you're already experienced in other languages.

I should preface this next response with:
I am not a programmer. I am not a developer. I don't necessarily want to be either. My job sometimes thrusts me into coding, sometimes I get to pick my language, and sometimes I don't.
By no means am I adept at any programming language, I am better in some than others though.
It has been my understanding for many years that language should be immaterial when programming. You should know the concepts, and how to work with data and the logic behind something rather than the syntax. Syntax is something that if you have a strong programming background can be picked up in a few weeks and become proficient in a month or two.
There is a lot about programming that I do not know or understand that I probably should, but those are things that if I continue to do it I should pick up with time.

I understand your point with this, but I don't fully agree with it. Reading through a bunch of pages on the Pythonic way of coding, I found that I don't necessarily agree with the Pythonic way. I would prefer not to code in a fashion that requires a ton of re-factoring to port my code into another language (not that I plan to any time soon). I can see the benefits of using Python specific coding practices to enhance and speed up processing using Python, but I don't necessarily want to be pigeon holed into coding just in Python. The thing that I read the most is that in Perl there are multiple ways of doing things, while in Python there is only 1 way. I could be misinterpreting some of what I read as I do not understand some of the more advanced ways of programming.
I chose Python originally because I didn't want to have to code in .NET for Windows, and bash or awk in *NIX, also I chose it because I liked the fact that it forced indentation to make more readable code.
Although I do agree that I should not try to encapsulate C or Java code in Python. In theory I should be able to find a healthy medium to live in.

setrofim wrote:
KHarvey wrote:As far as the code goes, is that the proper way to do the comparisons?
Where I assign a variable to the key and then do an "in" to see if the key exists and then create another list/dictionary once again based off the variable from the key in the previous list / dictionary?

You shouldn't be using indexes all over the place, and explicitly checking if the result of find() is -1 is unpythonic; apart from that looks more or less right. Your code has bigger issues at the moment. You should clean it up before asking about it's structure.

I kind of understand what I am supposed to clean up, but I am not really sure. It sounds like most of your suggestions have been to change my coding habits. Some of which I can agree with and change, and others that I don't necessarily agree with and will probably leave as is. In theory what I have done is cleanup, but I will let everyone else be the judge of that.

So going back to coding structure with some changes to the way I code, here is my new example:
Code: Select all
#Function to walk an SNMP tree and assign to a return list
def snmp_walk(version, vlan, ip, oid):
   oid_vars = []
   snmp_command = (
         "snmpwalk -v %s -c %s%s %s %s"
         %(version, strSNMPCommunity, vlan, ip, oid)
   )
   snmp_mac = subprocess.Popen(
         snmp_command, stdin=None, stdout=-1, stderr=None, shell=True
   )
   while True:
      read_line = snmp_mac.stdout.readline()
      if not read_line: break
      oid_vars.append(read_line.strip())
   return oid_vars

#Function to parse SNMP information based on the OID, what part of the OID is required, and the type of OID
def snmp_oid(oid_var, oid_parse, oid_type):
   oid_var_parsed = ""
   if oid_type in oid_var:
      oid_raw = oid_var.split(oid_type)
      oids = oid_raw[0].split(".")
      oid_type_parse = oids[oid_parse:]
      oid_parsed = ".".join(oids[oid_parse:]).strip()
      var = oid_var.split(oid_type)[1].strip()
      oid_var_parsed = (oid_parsed, var)
   return oid_var_parsed


snmp_mac_bp_ii_pn_dict = {"10.1.15.25":[]}
snmp_oid_macs = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.1")
snmp_oid_bps = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.2")
snmp_oid_iis = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.1.4.1.2")
snmp_oid_pns = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.31.1.1.1.1")
snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_macs)
snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_bps)
snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_iis)
snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_pns)

for ip in snmp_mac_bp_ii_pn_dict.keys():
   oid_macs = snmp_mac_bp_ii_pn_dict[ip][0]
   oid_bps = snmp_mac_bp_ii_pn_dict[ip][1]
   oid_iis = snmp_mac_bp_ii_pn_dict[ip][2]
   oid_pns = snmp_mac_bp_ii_pn_dict[ip][3]
   oid_macs_parsed = {}
   oid_bps_parsed = {}
   oid_iis_parsed = {}
   oid_pns_parsed = {}
   for oid_mac in oid_macs:
      oid_mac_parsed = snmp_oid(oid_mac, -6, "= Hex-STRING:")
      oid_macs_parsed.update({oid_mac_parsed[0]:oid_mac_parsed[1]})
   for oid_bp in oid_bps:
      oid_bp_parsed = snmp_oid(oid_bp, -6, "= INTEGER:")
      oid_bps_parsed.update({oid_bp_parsed[0]:oid_bp_parsed[1]})
   for oid_ii in oid_iis:
      oid_ii_parsed = snmp_oid(oid_ii, -1, "= INTEGER:")
      oid_iis_parsed.update({oid_ii_parsed[0]:oid_ii_parsed[1]})
   for oid_pn in oid_pns:
      oid_pn_parsed = snmp_oid(oid_pn, -1, "= STRING:")
      oid_pns_parsed.update({oid_pn_parsed[0]:oid_pn_parsed[1]})
   for mac_key, mac_address in oid_macs_parsed.iteritems():
      print mac_key
      if mac_key in oid_bps_parsed:
         if oid_bps_parsed[mac_key] in oid_iis_parsed:
            if oid_iis_parsed[oid_bps_parsed[mac_key]] in oid_pns_parsed:
               print oid_pns_parsed[oid_iis_parsed[oid_bps_parsed[mac_key]]]
               print mac_address


This is not completed code as I will need to add a few more loops into the code and then add in the final list that gets inserted into the database. This is more of an (working) example of the changes that were suggested and how I interpreted them. Some things are still a little beyond my coding skills, so some of the code looks similar.

I figured out a more efficient way of pulling the information from SNMP, which has shaved around 10 - 12 minutes off of the run time. When doing all the parsing and searching inline it takes longer to collect the information from the switches. By assigning them to dictionaries and lists and processing out of line it puts less stress on my network, and the code runs quite a bit quicker.

I created 2 new functions to pull information, and to parse information from SNMP so I am not duplicating a bunch of code. I also assigned a bunch of variables that I normally would not assign to avoid using too many indexes. Although at the end of the code I did end up using a bunch of indexes rather than variables. At that point I figured the code flowed fairly well and that reading through it it was simple to figure out.

For the Hungarian Notation comment that I had above that I would expound upon. One thing that I struggled with was when I was breaking out all (most of) of my indexed variables I started running into some issues with names, and my variables all started to look the same. This wasn't necessarily about not using Hungarian Notation, but had to do with my variable names as they all pretty much were the exact same variable, just parsed in a different way. There is probably a better way, so I am open to suggestions :)

I still think that I have a long way to go, but hopefully I am making progress in the proper direction. What do you think?
Thank you setrofim for the help and advice you have been providing.
KHarvey
 
Posts: 34
Joined: Tue Mar 19, 2013 5:13 pm
Location: US

Re: Inefficient Code Assistance

Postby setrofim » Thu Mar 28, 2013 9:05 am

KHarvey wrote:Garbage collection in Python is something that I need to look at a little more closely. Mainly to make sure things don't stay resident in memory even after the Python script / process has completed.

That will never happen, and it has nothing to do with garbage collection. The operating system allocates memory to a processes to be used for things like variables. When the process terminates, that memory is freed. This is the responsibility of the operating system and not that of the process. This is just as true for e.g. for C/C++ programs that (usually) run without garbage collectors. No much how badly a crappy C process leaks memory, that memory will always be freed when the process terminates (or is killed). The only exception to that are shared memory chunks that are allocated outside of the process' heap. But this will only be allocated through an explicit API and will then be managed like any other external resource; it will never happen during variable allocation.

KHarvey wrote:I now understand the difference between Apps Hungarian Notation and System Hungarian Notation. I still whole heartily agree with Hungarian Notation in certain circumstances (Apps).

KHarvey wrote:To me this is just replacing my prefix with a suffix. I think that most of my variables contain the info needed, I was just adding a little more to them at the beginning rather than the ending.

Limiting the lifetime of your variables greatly reduces the need to annotate them with such information. Sometimes it is still useful and you should do it. However, and this is the point, Hungarian notation advocates systematic use of archaic encoding system for this purpose. Different people end up varying the notation slightly, and an index becomes prefixed (or postfixed) with "idx" or "ix" or sometimes just "i". It quickly becomes very confusing for people unfamiliar with all the conventions (or for yourself coming back to the code a few months down the line -- are you gonna remember that "us" means "unsafe"). You should incorporate this information into variable names more organically, not through some arbitrary convention that mandates a prefix (or a postfix). So "start_date" and "date_of_birth" rather than "dtStart" and "dtBirth" (seriously, just look at these: which are clearer to you?).

As an other example, Wikipedia gives rwPosition as an illustration of Apps version of notation:
Wikipedia wrote:rwPosition : variable represents a row ("rw");

Really? To me this reads like "row position" and if I were reading an unfamiliar code line, my first thought would be that the variable represents a position within a row (or perhaps position of the row within a table, i.e. an index). Just call the variable position_row and there is no confusion.


KHarvey wrote: A good example that I read was using it to determine safe and unsafe variables. So something that has been cleaned for use in HTML or in SQL. Having a prefix to identify that a variable has been cleaned I believe is good practice as it can help to limit security threats.

Actually, this is a good example of Hungarian notation being use as crutch to deal with bad code. Good practice in this case is to factor the code that does the SQL query (or code that adds stuff to HTML) into a separate function that takes the value to be added and the database query/HTML document it is to be added to as inputs. You then assume that all values passed into that function are unsafe and sanitize them within the function just before executing the query or adding the value to the HTML. You then don't need to worry about safe/unsafe distinction anywhere else in your code. In fact, Python's DB APIs do exactly that -- you pass the query you want to run and the values you want to run it with as separate arguments to the query() method of the DB connection. The query() method then sanitizes the values for you.

KHarvey wrote:Reading through my Microsoft books it looks as though it is still good practice to do this, or rather all of their examples still do this.

Yes, because when the authors of those books were young software engineers, Hungarian notation was drilled into them; an now, they are passing on the pain (I think Code Complete that I mentioned was also published by MS Press).

KHarvey wrote:Reading through the PEP it appears that camel case is still used.

Yes, it is used for class names, but not variable or function names. Also note that there is a slight variation in terminology. What the PEP calls CamelCase is more commonly know as "title case", and what it calls mixedCase is also known as "camel case". The distinction being in the capitalization of the fist letter.

KHarvey wrote:The two options that I was given was to use a try except, or check for NoneType.

try/except is the pythonic way of doing this -- "It is better to seek forgiveness then to ask permission". Not everyone follow this though.


KHarvey wrote:Reading through a bunch of pages on the Pythonic way of coding, I found that I don't necessarily agree with the Pythonic way.

In this case, which is more likely: that the experts that came up with the Pythonic way are wrong; or that you are? I'm not advocating religious indoctrination here. You should definitely question things that don't make sense. And you don't need to incorporate everything it says into your code all the time -- nobody's code is 100% Pythonic (probably not even Guido's). But you should think carefully before dismissing something that has been established as good practice by people who know the language (and programming in general) better than you do.

KHarvey wrote: I would prefer not to code in a fashion that requires a ton of re-factoring to port my code into another language (not that I plan to any time soon).

This is unwise. Different languages are different, and porting effort is unavoidable. Attempting to write code that is as language-generic as possible will result in bad code that is unmaintainable (in any of the languages). It is especially stupid to be worrying about this when there are no plans to port it. Porting happens a lot less often then you'd think. For applications, anyway; for libraries it is more common, but even then "porting" usually involved re-implementing the library in the new language. When you're writing Python, write Python; when you're writing C, write C; when you're writing Java... you get the picture.
setrofim
 
Posts: 288
Joined: Mon Mar 04, 2013 7:52 pm

Re: Inefficient Code Assistance

Postby metulburr » Thu Mar 28, 2013 9:36 am

adding on to the variable names:
i just did this too. I made a variable "cigs" and went on to use it over and over. Sometime later it started getting confusing because there were different integers regarding cigs (related). I spent a lot of time getting errors or unexpected results because of that confusion, and spent a lot of time reverse looking at what that variable actually was suppose to represent. So i renamed the variable "total_cigs_not_smoked". Yes it is long., but i will never get confused about that variable again, now or in the future.

I dont really like camelcase, not sure why. And Hungarian notation of any form seems to me more confusing. I have always liked undercore as spaces as somewhat a small sentance as a variable name that describes what that variable is. Long but effective.
New Users, Read This
OS Ubuntu 14.04, Arch Linux, Gentoo, Windows 7/8
https://github.com/metulburr
steam
User avatar
metulburr
 
Posts: 1321
Joined: Thu Feb 07, 2013 4:47 pm
Location: Elmira, NY

Re: Inefficient Code Assistance

Postby micseydel » Thu Mar 28, 2013 9:42 am

If you have an IDE with autocomplete the long variable names aren't necessarily a bad thing. They can be annoying when you have deep indentation, or use them in longer one-liners, and you exceed the 80 character line thing.
Join the #python-forum IRC channel on irc.freenode.net!
User avatar
micseydel
 
Posts: 1131
Joined: Tue Feb 12, 2013 2:18 am
Location: Mountain View, CA

Re: Inefficient Code Assistance

Postby setrofim » Thu Mar 28, 2013 10:04 am

KHarvey wrote:So going back to coding structure with some changes to the way I code, here is my new example:
Code: Select all
#Function to walk an SNMP tree and assign to a return list
def snmp_walk(version, vlan, ip, oid):
        oid_vars = []
        snmp_command = (
                        "snmpwalk -v %s -c %s%s %s %s"
                        %(version, strSNMPCommunity, vlan, ip, oid)
        )
        snmp_mac = subprocess.Popen(
                        snmp_command, stdin=None, stdout=-1, stderr=None, shell=True
        )
        while True:
                read_line = snmp_mac.stdout.readline()
                if not read_line: break
                oid_vars.append(read_line.strip())
        return oid_vars

#Function to parse SNMP information based on the OID, what part of the OID is required, and the type of OID
def snmp_oid(oid_var, oid_parse, oid_type):
        oid_var_parsed = ""
        if oid_type in oid_var:
                oid_raw = oid_var.split(oid_type)
                oids = oid_raw[0].split(".")
                oid_type_parse = oids[oid_parse:]
                oid_parsed = ".".join(oids[oid_parse:]).strip()
                var = oid_var.split(oid_type)[1].strip()
                oid_var_parsed = (oid_parsed, var)
        return oid_var_parsed


snmp_mac_bp_ii_pn_dict = {"10.1.15.25":[]}
snmp_oid_macs = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.1")
snmp_oid_bps = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.2")
snmp_oid_iis = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.1.4.1.2")
snmp_oid_pns = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.31.1.1.1.1")
snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_macs)
snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_bps)
snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_iis)
snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_pns)

for ip in snmp_mac_bp_ii_pn_dict.keys():
        oid_macs = snmp_mac_bp_ii_pn_dict[ip][0]
        oid_bps = snmp_mac_bp_ii_pn_dict[ip][1]
        oid_iis = snmp_mac_bp_ii_pn_dict[ip][2]
        oid_pns = snmp_mac_bp_ii_pn_dict[ip][3]
        oid_macs_parsed = {}
        oid_bps_parsed = {}
        oid_iis_parsed = {}
        oid_pns_parsed = {}
        for oid_mac in oid_macs:
                oid_mac_parsed = snmp_oid(oid_mac, -6, "= Hex-STRING:")
                oid_macs_parsed.update({oid_mac_parsed[0]:oid_mac_parsed[1]})
        for oid_bp in oid_bps:
                oid_bp_parsed = snmp_oid(oid_bp, -6, "= INTEGER:")
                oid_bps_parsed.update({oid_bp_parsed[0]:oid_bp_parsed[1]})
        for oid_ii in oid_iis:
                oid_ii_parsed = snmp_oid(oid_ii, -1, "= INTEGER:")
                oid_iis_parsed.update({oid_ii_parsed[0]:oid_ii_parsed[1]})
        for oid_pn in oid_pns:
                oid_pn_parsed = snmp_oid(oid_pn, -1, "= STRING:")
                oid_pns_parsed.update({oid_pn_parsed[0]:oid_pn_parsed[1]})
        for mac_key, mac_address in oid_macs_parsed.iteritems():
                print mac_key
                if mac_key in oid_bps_parsed:
                        if oid_bps_parsed[mac_key] in oid_iis_parsed:
                                if oid_iis_parsed[oid_bps_parsed[mac_key]] in oid_pns_parsed:
                                        print oid_pns_parsed[oid_iis_parsed[oid_bps_parsed[mac_key]]]
                                        print mac_address



This is much better. Though, as I said in the previous post, in variables such as "snmp_mac_bp_ii_pn_dict" are you going to remember what "bp", "ii" and "pn" mean six months from now? And the fact that it is a dict is obvious in how it is used, so there is no point in cluttering the name with that information.

A few comments:
  • A cleaner way of doing this:
    Code: Select all
                    oid_macs_parsed.update({oid_mac_parsed[0]:oid_mac_parsed[1]})

    is
    Code: Select all
                    oid_macs_parsed[oid_mac_parsed[0]] = oid_mac_parsed[1]

    update() method is useful when you want to add an existing dict, but there is no point in using it for a single key-value pair.
  • Comments documenting functions like this:
    Code: Select all
    #Function to walk an SNMP tree and assign to a return list
    def snmp_walk(version, vlan, ip, oid):
        # ....

    Are better done like this:
    Code: Select all
    def snmp_walk(version, vlan, ip, oid):
        '''Function to walk an SNMP tree and assign to a return list'''
        # ....

    These are the docstrings that I mentioned before. The advantage of using them over regular comments is that you can later query them in the interactive interpreter, getting immediate function help. They can also be extracted by documentation tools to automatically generate API reference.
  • Going back to advice about declaring variables close to where they are used, this
    Code: Select all
            oid_macs_parsed = {}
            oid_bps_parsed = {}
            oid_iis_parsed = {}
            oid_pns_parsed = {}

            for oid_mac in oid_macs:
                    # ...
           

            for oid_bp in oid_bps:
                    # ...

            for oid_ii in oid_iis:
                    # ...

            for oid_pn in oid_pns:
                    # ...

    would be better structured as
    Code: Select all
            oid_macs_parsed = {}
            for oid_mac in oid_macs:
                    # ...
           

            oid_bps_parsed = {}
            for oid_bp in oid_bps:
                    # ...

            oid_iis_parsed = {}
            for oid_ii in oid_iis:
                    # ...

            oid_pns_parsed = {}
            for oid_pn in oid_pns:
                    # ...

    but actually...
  • This code:
    Code: Select all
            oid_macs_parsed = {}
            for oid_mac in oid_macs:
                    oid_mac_parsed = snmp_oid(oid_mac, -6, "= Hex-STRING:")
                    oid_macs_parsed.update({oid_mac_parsed[0]:oid_mac_parsed[1]})

    Can be replaced with a single line:
    Code: Select all
            oid_macs_parsed = dict(snmp_oid(om, -6, "= Hex-STRING:") for om in oid_macs)

    This is taking advantage of the fact that you can construct a dict using a list of key-value pairs. Not only is this more concise, but it will execute faster than a for loop.
  • Similarly, rather than initialising an empty list and then appending:
    Code: Select all
    snmp_mac_bp_ii_pn_dict = {"10.1.15.25":[]}
    snmp_oid_macs = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.1")
    snmp_oid_bps = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.2")
    snmp_oid_iis = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.1.4.1.2")
    snmp_oid_pns = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.31.1.1.1.1")
    snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_macs)
    snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_bps)
    snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_iis)
    snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_pns)

    just do
    Code: Select all
    snmp_mac_bp_ii_pn_dict = {}
    snmp_mac_bp_ii_pn_dict["10.1.15.25"] = [
                    snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.1"),
                    snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.2"),
                    snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.1.4.1.2"),
                    snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.31.1.1.1.1")
    ]
  • A good practice in Python, To do this for code at the top level (code that is not in a class or a function):
    Code: Select all
    if __name__ == '__main__':
            snmp_mac_bp_ii_pn_dict = {"10.1.15.25":[]}
            snmp_oid_macs = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.1")
            snmp_oid_bps = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.2")
            # ...

    This prevents the code from being accidentally executed if you later import this .py file as module in anther script. Of course, with some files, it may be obvious that it would only be run as a script an not used as a module, but again, this is the case of getting into good habits early.
  • This maybe getting a little advance, but I would recommend you read up on classes. The fact that you have a lot of variables and functions called snmp_* suggests that there should be a Snmp class. Classes will give you better way of associating various data and functions that operate on that data than naming conventions.
Last edited by setrofim on Thu Mar 28, 2013 11:42 am, edited 1 time in total.
setrofim
 
Posts: 288
Joined: Mon Mar 04, 2013 7:52 pm

Re: Inefficient Code Assistance

Postby setrofim » Thu Mar 28, 2013 10:09 am

micseydel wrote: They can be annoying when you have deep indentation, or use them in longer one-liners, and you exceed the 80 character line thing.

True; and this is the reason why the 80 column rule is often ignored or is "softened" to be 100 columns (or 120, or...). It is important to keep in mind that there is value in keeping your lines short, but imposing an arbitrary limits such as the 80 column rule just encourages using shorter, less descriptive variable names (on the other hand, it also encourages avoiding deep nesting -- which is a good thing).
setrofim
 
Posts: 288
Joined: Mon Mar 04, 2013 7:52 pm

Re: Inefficient Code Assistance

Postby micseydel » Thu Mar 28, 2013 10:21 am

80 chars is how wide my terminals open up by default, and for when I use a weird tty as my terminal. In fact, if I'm editing code in a weird tty, I used vim and with "set number" enabled I want my lines even shorter than 80. So as far as arbitrary rules, if you're in a workplace where you use an IDE that you can expand, then whatever the reasonable expansion is a good rule, and for someone like me who is probably unusual in habits, 75 is better than 80, if the code can be 1000+ lines.
Join the #python-forum IRC channel on irc.freenode.net!
User avatar
micseydel
 
Posts: 1131
Joined: Tue Feb 12, 2013 2:18 am
Location: Mountain View, CA

Re: Inefficient Code Assistance

Postby setrofim » Thu Mar 28, 2013 10:26 am

micseydel wrote:80 chars is how wide my terminals open up by default

Only if you don't change the default ;). Any good terminal emulator will let you do that. I usually up it to 100 columns, as it still allows me to have two side-by-side; and those extra 20 cols make a big difference. As I've mentioned earlier in this thread, the 80 column thing is an accident of history; there is no reason to limit yourself to that now.
setrofim
 
Posts: 288
Joined: Mon Mar 04, 2013 7:52 pm

Re: Inefficient Code Assistance

Postby micseydel » Thu Mar 28, 2013 10:42 am

Actually, 80+80 is my screen width, and with the scroll bar they overlap with two terminals at 80 width. Normally when I code, I have a terminal and Sublime Text, with Sublime at 80 or a bit more and the terminal overlapping.
Join the #python-forum IRC channel on irc.freenode.net!
User avatar
micseydel
 
Posts: 1131
Joined: Tue Feb 12, 2013 2:18 am
Location: Mountain View, CA

Re: Inefficient Code Assistance

Postby setrofim » Thu Mar 28, 2013 10:59 am

micseydel wrote:Actually, 80+80 is my screen width, and with the scroll bar they overlap with two terminals at 80 width. Normally when I code, I have a terminal and Sublime Text, with Sublime at 80 or a bit more and the terminal overlapping.

That's fair enough then :). Still, the important thing here is that the column width is adapted to your monitor and your working process, not the precise number 80. Most modern monitors come with resolutions that allow having wider windows side-by-side. On my laptop, I neatly side step the issue by running Awesome WM (a tiling window manager) that expands the windows to fill the available space (I believe it works out as something like 94 columns per window, as the resolution on that laptop isn't great).
setrofim
 
Posts: 288
Joined: Mon Mar 04, 2013 7:52 pm

Re: Inefficient Code Assistance

Postby KHarvey » Thu Mar 28, 2013 2:22 pm

setrofim wrote:
KHarvey wrote:I now understand the difference between Apps Hungarian Notation and System Hungarian Notation. I still whole heartily agree with Hungarian Notation in certain circumstances (Apps).

KHarvey wrote:To me this is just replacing my prefix with a suffix. I think that most of my variables contain the info needed, I was just adding a little more to them at the beginning rather than the ending.

Limiting the lifetime of your variables greatly reduces the need to annotate them with such information. Sometimes it is still useful and you should do it. However, and this is the point, Hungarian notation advocates systematic use of archaic encoding system for this purpose. Different people end up varying the notation slightly, and an index becomes prefixed (or postfixed) with "idx" or "ix" or sometimes just "i". It quickly becomes very confusing for people unfamiliar with all the conventions (or for yourself coming back to the code a few months down the line -- are you gonna remember that "us" means "unsafe"). You should incorporate this information into variable names more organically, not through some arbitrary convention that mandates a prefix (or a postfix). So "start_date" and "date_of_birth" rather than "dtStart" and "dtBirth" (seriously, just look at these: which are clearer to you?).

Honestly both of them are easy for me to read, but I guess that I have just spent too much time in short hand. ;)

setrofim wrote:As an other example, Wikipedia gives rwPosition as an illustration of Apps version of notation:
Wikipedia wrote:rwPosition : variable represents a row ("rw");

Really? To me this reads like "row position" and if I were reading an unfamiliar code line, my first thought would be that the variable represents a position within a row (or perhaps position of the row within a table, i.e. an index). Just call the variable position_row and there is no confusion.

I see now why people are against Hungarian Notation in any fashion. It is the encoding or short handing of the variables. The shorter variables were easier to keep inside the 80 char rule. That makes sense. My only issue with longer variables is it takes me longer to type or I have to copy and paste. Once again, I normally don't use an IDE as I am not always working on the same computer.

setrofim wrote:
KHarvey wrote: A good example that I read was using it to determine safe and unsafe variables. So something that has been cleaned for use in HTML or in SQL. Having a prefix to identify that a variable has been cleaned I believe is good practice as it can help to limit security threats.

Actually, this is a good example of Hungarian notation being use as crutch to deal with bad code. Good practice in this case is to factor the code that does the SQL query (or code that adds stuff to HTML) into a separate function that takes the value to be added and the database query/HTML document it is to be added to as inputs. You then assume that all values passed into that function are unsafe and sanitize them within the function just before executing the query or adding the value to the HTML. You then don't need to worry about safe/unsafe distinction anywhere else in your code. In fact, Python's DB APIs do exactly that -- you pass the query you want to run and the values you want to run it with as separate arguments to the query() method of the DB connection. The query() method then sanitizes the values for you.

I actually disagree with you here. This may not be very applicable to Python, or rather I have not had to write a piece of code that requires me to sanitize it yet. But in web programming, if a user submits a form you should check all the user submitted variables. For example if you have a drop down menu, very that the value submitted for the drop down menu is actually an option in said drop down menu. I usually do this verification at the top of my code rather than the bottom where I do my SQL updates or insertions. The main reason is there isn't any point to me processing the rest of the page for an XSS attack, especially if it is a long running script. That is normally where I assign the Clean variable at the top of my code when I do the verification. But I now see your point of rather than saying us or s or cln, I should just name the variable clean_username.

setrofim wrote:
KHarvey wrote:Reading through my Microsoft books it looks as though it is still good practice to do this, or rather all of their examples still do this.

Yes, because when the authors of those books were young software engineers, Hungarian notation was drilled into them; an now, they are passing on the pain (I think Code Complete that I mentioned was also published by MS Press).

This is why I say it does not appear to be a consensus, as there are still a lot of books and programmers that advocate this style. I will probably continue to use some short hand in my code, but I will try to limit its use.

setrofim wrote:
KHarvey wrote:The two options that I was given was to use a try except, or check for NoneType.

try/except is the pythonic way of doing this -- "It is better to seek forgiveness then to ask permission". Not everyone follow this though.

This is a little backwards to me, but I see your point. I'll have to think about this piece a little bit more. Mainly from secure coding practices you verify then process, not process and error on fail. I guess it depends on context, but I think it would be better to be consistent rather than switching logic all the time. I will think about this some more.

setrofim wrote:
KHarvey wrote:Reading through a bunch of pages on the Pythonic way of coding, I found that I don't necessarily agree with the Pythonic way.

In this case, which is more likely: that the experts that came up with the Pythonic way are wrong; or that you are? I'm not advocating religious indoctrination here. You should definitely question things that don't make sense. And you don't need to incorporate everything it says into your code all the time -- nobody's code is 100% Pythonic (probably not even Guido's). But you should think carefully before dismissing something that has been established as good practice by people who know the language (and programming in general) better than you do.

This was why I changed my code. I'm not saying that the Pythonic way of coding is wrong, I'm just not sure that is the style for me. I still need to learn more about the language and programming in general and make adjustments to my code as I learn.

setrofim wrote:
KHarvey wrote: I would prefer not to code in a fashion that requires a ton of re-factoring to port my code into another language (not that I plan to any time soon).

This is unwise. Different languages are different, and porting effort is unavoidable. Attempting to write code that is as language-generic as possible will result in bad code that is unmaintainable (in any of the languages). It is especially stupid to be worrying about this when there are no plans to port it. Porting happens a lot less often then you'd think. For applications, anyway; for libraries it is more common, but even then "porting" usually involved re-implementing the library in the new language. When you're writing Python, write Python; when you're writing C, write C; when you're writing Java... you get the picture.

I'm not saying that I should be able to copy and paste my code, I'm more referring to the capability of being able to take the logic and pass it over to another language. In school we were taught one language (for the most part), but once I hit the real world and the industry I found the programmers that could only use one language were lacking (I've got a couple of programmers that I am dealing with now that are like that). The programmers that could pick up what ever they needed always advocated being able to build your code around the logic and not necessarily the language / syntax. There are of course exceptions. One of those programmers was in a similar discussion to this one not that long ago about some of his Perl scripts. Basically he wasn't programming it the Perl(thonic) way. But then again the code wasn't originally designed in Perl, it was designed in PHP and ported over to Perl.
Now in all fairness some of the programmers that I know that do code in multiple languages aren't that good of programmers such as myself :P .
This set of code that I am working on now is a port from VBScript, to .NET, to PHP, to now Python. Once again, I am not a programmer, I am a coder or scripter at best.

micseydel wrote:If you have an IDE with autocomplete the long variable names aren't necessarily a bad thing. They can be annoying when you have deep indentation, or use them in longer one-liners, and you exceed the 80 character line thing.

I never really got into IDE's much. I had to use them for Microsoft stuff (.NET), but most of my code has always been done in a text editor (notepad, vi). By the way, I hated Visual Studio.
The main reason I don't use IDE's is that I reinstall my computer, or move between computers so often that I got tired of customizing them, so I went to a more generic configuration. Although, I am building my new dev environment on a USB stick so that I can just boot it where ever I go, but I don't know what IDE's that OS has. I may end up trying to port one over to it though.
KHarvey
 
Posts: 34
Joined: Tue Mar 19, 2013 5:13 pm
Location: US

Re: Inefficient Code Assistance

Postby setrofim » Thu Mar 28, 2013 3:07 pm

KHarvey wrote: My only issue with longer variables is it takes me longer to type or I have to copy and paste. Once again, I normally don't use an IDE as I am not always working on the same computer.

I don't use an IDE for Python, and I develop on about four different machines. A decent text editor will give you basic autocompletion (for symbols in the same file). Besides you should be optimizing for reading no writing -- if takes longer to type but makes the code a bit clearer, it's a good trade off.


KHarvey wrote:
setrofim wrote:
KHarvey wrote: A good example that I read was using it to determine safe and unsafe variables. So something that has been cleaned for use in HTML or in SQL. Having a prefix to identify that a variable has been cleaned I believe is good practice as it can help to limit security threats.

Actually, this is a good example of Hungarian notation being use as crutch to deal with bad code. Good practice in this case is to factor the code that does the SQL query (or code that adds stuff to HTML) into a separate function that takes the value to be added and the database query/HTML document it is to be added to as inputs. You then assume that all values passed into that function are unsafe and sanitize them within the function just before executing the query or adding the value to the HTML. You then don't need to worry about safe/unsafe distinction anywhere else in your code. In fact, Python's DB APIs do exactly that -- you pass the query you want to run and the values you want to run it with as separate arguments to the query() method of the DB connection. The query() method then sanitizes the values for you.

I actually disagree with you here. This may not be very applicable to Python, or rather I have not had to write a piece of code that requires me to sanitize it yet. But in web programming, if a user submits a form you should check all the user submitted variables. For example if you have a drop down menu, very that the value submitted for the drop down menu is actually an option in said drop down menu. I usually do this verification at the top of my code rather than the bottom where I do my SQL updates or insertions. The main reason is there isn't any point to me processing the rest of the page for an XSS attack, especially if it is a long running script. That is normally where I assign the Clean variable at the top of my code when I do the verification. But I now see your point of rather than saying us or s or cln, I should just name the variable clean_username.

Web form validation is deferent; here you should be validating both on the client side and on the server side. This does not invalidate my point, however. The important thing is to validate consistently -- either right as you get the input or just before submitting a query. And in both cases, sanitization is best outsourced to a dedicated function. Doing that, you don't need to worry about sanitization status of your variables in the rest of your code -- either they're never sanitized (in which case you sanitize before submitting to the database, or they are always sanitized (in which case you must make sure that is the case by carefully sanitizing on each user input). In Python, it makes sense to sanitize before running DB query, because then you only have to do it in one place; in other situations it might make sense to sanitize immediately as you get input form the user. But it is always either one or the other; and in both cases, the sanitization status of your variables is unambiguous.

KHarvey wrote:I'm not saying that I should be able to copy and paste my code, I'm more referring to the capability of being able to take the logic and pass it over to another language.

So am I. Different languages are optimized for different things and have different philosophies on how things should be done. These things are as integral to the language as the language's syntax. So the same business logic implemented in different languages will look very differently.

KHarvey wrote:In school we were taught one language (for the most part), but once I hit the real world and the industry I found the programmers that could only use one language were lacking (I've got a couple of programmers that I am dealing with now that are like that).

For what it's worth, I regularly write Python, C#, C++ and R. If we were taking about implementing your code in C#, my suggestions on how to change it would be very different (though the underlying principles, such as writing in the language, my stance on Hungarian notation, etc, are obviously language agnostic).

KHarvey wrote:The programmers that could pick up what ever they needed always advocated being able to build your code around the logic and not necessarily the language / syntax.

That is very good advice, but it does not contradict my point. As an example, this is quicksort implemented in C. And these are quicksort implementations in Python (particularly, look at the last one). The code is completely different (port of Python code to C is not obvious). The underlying logic (the quicksort algorithm) is exactly the same and is not driven by either language.
setrofim
 
Posts: 288
Joined: Mon Mar 04, 2013 7:52 pm

Re: Inefficient Code Assistance

Postby KHarvey » Thu Mar 28, 2013 3:14 pm

setrofim wrote:
KHarvey wrote:So going back to coding structure with some changes to the way I code, here is my new example:
Code: Select all
#Function to walk an SNMP tree and assign to a return list
def snmp_walk(version, vlan, ip, oid):
        oid_vars = []
        snmp_command = (
                        "snmpwalk -v %s -c %s%s %s %s"
                        %(version, strSNMPCommunity, vlan, ip, oid)
        )
        snmp_mac = subprocess.Popen(
                        snmp_command, stdin=None, stdout=-1, stderr=None, shell=True
        )
        while True:
                read_line = snmp_mac.stdout.readline()
                if not read_line: break
                oid_vars.append(read_line.strip())
        return oid_vars

#Function to parse SNMP information based on the OID, what part of the OID is required, and the type of OID
def snmp_oid(oid_var, oid_parse, oid_type):
        oid_var_parsed = ""
        if oid_type in oid_var:
                oid_raw = oid_var.split(oid_type)
                oids = oid_raw[0].split(".")
                oid_type_parse = oids[oid_parse:]
                oid_parsed = ".".join(oids[oid_parse:]).strip()
                var = oid_var.split(oid_type)[1].strip()
                oid_var_parsed = (oid_parsed, var)
        return oid_var_parsed


snmp_mac_bp_ii_pn_dict = {"10.1.15.25":[]}
snmp_oid_macs = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.1")
snmp_oid_bps = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.2")
snmp_oid_iis = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.1.4.1.2")
snmp_oid_pns = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.31.1.1.1.1")
snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_macs)
snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_bps)
snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_iis)
snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_pns)

for ip in snmp_mac_bp_ii_pn_dict.keys():
        oid_macs = snmp_mac_bp_ii_pn_dict[ip][0]
        oid_bps = snmp_mac_bp_ii_pn_dict[ip][1]
        oid_iis = snmp_mac_bp_ii_pn_dict[ip][2]
        oid_pns = snmp_mac_bp_ii_pn_dict[ip][3]
        oid_macs_parsed = {}
        oid_bps_parsed = {}
        oid_iis_parsed = {}
        oid_pns_parsed = {}
        for oid_mac in oid_macs:
                oid_mac_parsed = snmp_oid(oid_mac, -6, "= Hex-STRING:")
                oid_macs_parsed.update({oid_mac_parsed[0]:oid_mac_parsed[1]})
        for oid_bp in oid_bps:
                oid_bp_parsed = snmp_oid(oid_bp, -6, "= INTEGER:")
                oid_bps_parsed.update({oid_bp_parsed[0]:oid_bp_parsed[1]})
        for oid_ii in oid_iis:
                oid_ii_parsed = snmp_oid(oid_ii, -1, "= INTEGER:")
                oid_iis_parsed.update({oid_ii_parsed[0]:oid_ii_parsed[1]})
        for oid_pn in oid_pns:
                oid_pn_parsed = snmp_oid(oid_pn, -1, "= STRING:")
                oid_pns_parsed.update({oid_pn_parsed[0]:oid_pn_parsed[1]})
        for mac_key, mac_address in oid_macs_parsed.iteritems():
                print mac_key
                if mac_key in oid_bps_parsed:
                        if oid_bps_parsed[mac_key] in oid_iis_parsed:
                                if oid_iis_parsed[oid_bps_parsed[mac_key]] in oid_pns_parsed:
                                        print oid_pns_parsed[oid_iis_parsed[oid_bps_parsed[mac_key]]]
                                        print mac_address



This is much better. Though, as I said in the previous post, in variables such as "snmp_mac_bp_ii_pn_dict" are you going to remember what "bp", "ii" and "pn" mean six months from now? And the fact that it is a dict is obvious in how it is used, so there is no point in cluttering the name with that information.

Honestly "bp", "ii" don't really have a meaning. They stand for Bridge Port and ifIndex but those don't really have a meaning in the code, they are just kind of a place holder. If I wanted to be accurate I would use the OID address of each of those, as that would be more accurate, although I think my variable name would go well above the 80 char pseudo limit. Six months from now, those variables will mean pretty much the exact same thing. This is why my code before had comments with the definitions of these variables:
Code: Select all
#Pull a list of the Bridge Ports in all VLANs
#.1.3.6.1.2.1.17.4.3.1.2.0.7.77.58.103.152 = INTEGER: 3

The .1.3.6.1.2.1.17.4.3.1.2.0.7.77.58.103.152 = INTEGER: 3 has way more meaning then the variable itself as it gives me a path and what to compare. If I come back to the code that will be the part that I care about.
The pn should probably be changed to port_name as that does have meaning. :oops:
The dict was added on as when I was looking for code examples every piece of code that I read had that in there. Well, to be more precise, every piece of code that hand variable names greater than 1 character. Although I didn't use that convention any where else in that piece of code, as I just added an "s" to the end to make it plural if it was a list or a dictionary.

setrofim wrote:A few comments:
  • A cleaner way of doing this:
    Code: Select all
                    oid_macs_parsed.update({oid_mac_parsed[0]:oid_mac_parsed[1]})

    is
    Code: Select all
                    oid_macs_parsed[oid_mac_parsed[0]] = oid_mac_parsed[1]

    update() method is useful when you want to add an existing dict, but there is no point in using it for a single key-value pair.

I see, so I should only use update() when I am submitting multiple values (a list or another dictionary). Does that go the same for append() with lists? Actually I can't think of the syntax to do that to a list off the top of my head.

setrofim wrote:
  • Comments documenting functions like this:
    Code: Select all
    #Function to walk an SNMP tree and assign to a return list
    def snmp_walk(version, vlan, ip, oid):
        # ....

    Are better done like this:
    Code: Select all
    def snmp_walk(version, vlan, ip, oid):
        '''Function to walk an SNMP tree and assign to a return list'''
        # ....

    These are the docstrings that I mentioned before. The advantage of using them over regular comments is that you can later query them in the interactive interpreter, getting immediate function help. They can also be extracted by documentation tools to automatically generate API reference.

  • I wouldn't have thought of putting the comments inside the actual function. But that is an easy fix. I will switch my comments over to docstrings for my functions. I normally use docstrings at the top of my code for a description of everything, but I hadn't done that for my fuctions.

    setrofim wrote:
  • Going back to advice about declaring variables close to where they are used, this
    Code: Select all
            oid_macs_parsed = {}
            oid_bps_parsed = {}
            oid_iis_parsed = {}
            oid_pns_parsed = {}

            for oid_mac in oid_macs:
                    # ...
           

            for oid_bp in oid_bps:
                    # ...

            for oid_ii in oid_iis:
                    # ...

            for oid_pn in oid_pns:
                    # ...

    would be better structured as
    Code: Select all
            oid_macs_parsed = {}
            for oid_mac in oid_macs:
                    # ...
           

            oid_bps_parsed = {}
            for oid_bp in oid_bps:
                    # ...

            oid_iis_parsed = {}
            for oid_ii in oid_iis:
                    # ...

            oid_pns_parsed = {}
            for oid_pn in oid_pns:
                    # ...

    but actually...
  • This code:
    Code: Select all
            oid_macs_parsed = {}
            for oid_mac in oid_macs:
                    oid_mac_parsed = snmp_oid(oid_mac, -6, "= Hex-STRING:")
                    oid_macs_parsed.update({oid_mac_parsed[0]:oid_mac_parsed[1]})

    Can be replaced with a single line:
    Code: Select all
            oid_macs_parsed = dict(snmp_oid(om, -6, "= Hex-STRING:") for om in oid_macs)

    This is taking advantage of the fact that you can construct a dict using a list of key-value pairs. Not only is this more concise, but it will execute faster than a for loop.

  • So the first part of moving the creation of the variables to just above the code block makes sense.
    Although I don't quite understand the point to the single line of code that you are proposing. Doesn't that single line do a for loop as well, but it is just inline?
    The refactoring to the single line seems to go against what we have been discussing above and more back to the way of when I was doing a bunch of things on a single line. I'm not really complaining just trying to make sure that I understand. There is probably a finesse to how complicated code is and over simplification, but I am more of a bazooka to kill a fly kind of guy :P

    setrofim wrote:
  • Similarly, rather than initialising an empty list and then appending:
    Code: Select all
    snmp_mac_bp_ii_pn_dict = {"10.1.15.25":[]}
    snmp_oid_macs = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.1")
    snmp_oid_bps = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.2")
    snmp_oid_iis = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.1.4.1.2")
    snmp_oid_pns = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.31.1.1.1.1")
    snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_macs)
    snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_bps)
    snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_iis)
    snmp_mac_bp_ii_pn_dict["10.1.15.25"].append(snmp_oid_pns)

    just do
    Code: Select all
    snmp_mac_bp_ii_pn_dict = {}
    snmp_mac_bp_ii_pn_dict["10.1.15.25"] = [
                    snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.1"),
                    snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.2"),
                    snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.1.4.1.2"),
                    snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.31.1.1.1.1")
    ]
  • A good practice in Python, To do this for code at the top level (code that is not in a class or a function):
    Code: Select all
    if __name__ == '__main__':
            snmp_mac_bp_ii_pn_dict = {"10.1.15.25":[]}
            snmp_oid_macs = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.1")
            snmp_oid_bps = snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.2")
            # ...

    This prevents the code from being accidentally executed if you later import this .py file as module in anther script. Of course, with some files, it may be obvious that it would only be run as a script an not used as a module, but again, this is the case of getting into good habits early.

  • As I stated this was not complete code, as I wanted to make sure I was on the right track before I finished writing it. There's nothing quite like the feeling of completing a script only to find you did it wrong and need to start over. ;)
    So in the case of snmp_mac_bp_ii_pn_dict (that's a lot to type) I just hard coded a single IP address into it, but it will contain multiple IP's. I was going to use a loop to populate the dictionary with the list of IP addresses. Something like:
    Code: Select all
    for ip_address in sql_query:
      snmp_mac_bp_ii_pn_dict = {ip_address:[]}

    Now I know that reusing the same statement over and over again is bad coding practice as now you have to change it in multiple places rather than just one. But is that also the case for lists? For example using your example and my loop I could do something like:
    Code: Select all
    snmp_walk_mac_bp_ii_pn = [
     snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.1"),
                    snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.2"),
                    snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.17.1.4.1.2"),
                    snmp_walk("2c", "@5", "10.1.15.25", ".1.3.6.1.2.1.31.1.1.1.1")
    ]
    for ip_address in sql_query:
      snmp_mac_bp_ii_pn_dict[ip_address] = snmp_walk_mac_bp_ii_pn

    But that doesn't seem quite right either. I think that maybe I should create the snmp_walk_mac_bp_ii_pn list and not assign it the snmp_mac_bp_ii_pn_dict keys, and just loop through it instead.

    Another piece that will factor in as well is my snmp_walk statements will be in a loop as well, as I will need to increment the VLAN (@5) based on the VLAN's assigned to the switches.
    Code: Select all
                    snmp_walk("2c", "@1", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.2"),
                    snmp_walk("2c", "@1", "10.1.15.25", ".1.3.6.1.2.1.17.1.4.1.2"),
                    snmp_walk("2c", "@1", "10.1.15.25", ".1.3.6.1.2.1.31.1.1.1.1")
                    snmp_walk("2c", "@2", "10.1.15.25", ".1.3.6.1.2.1.17.4.3.1.2"),
                    snmp_walk("2c", "@2", "10.1.15.25", ".1.3.6.1.2.1.17.1.4.1.2"),
                    snmp_walk("2c", "@2", "10.1.15.25", ".1.3.6.1.2.1.31.1.1.1.1")

    The @ number could possibly go all the way up to 999. In my case it tops out at 15.

    Originally I had planned on handling both of those (IP and VLAN) with for loops, and just moving the code from above in two indentations to fall under those for loops. But if I change my code the way that you are recommending, then my idea is probably incorrect.

    setrofim wrote:
  • This maybe getting a little advance, but I would recommend you read up on classes. The fact that you have a lot of variables and functions called snmp_* suggests that there should be a Snmp class. Classes will give you better way of associating various data and functions that operate on that data than naming conventions.

  • Honestly, classes are the thing that I struggle with the most in any language. I kind of understand them in the fact that a variable now has objects to use, and I am able to code using those objects (which is easier in my mind). My issue comes from I don't really know how to create the classes, or probably more precisely I don't know how to build the logic for my own classes. I have read through a bunch of tutorials and such but haven't been able to wrap my head around it yet. I will try reading the link that you supplied again and see if it makes more sense to me now.
    If I am able to grasp classes then I think a lot of the things that I do will be cleaned up by them, and my code should look much better.
    KHarvey
     
    Posts: 34
    Joined: Tue Mar 19, 2013 5:13 pm
    Location: US

    Re: Inefficient Code Assistance

    Postby KHarvey » Thu Mar 28, 2013 3:47 pm

    setrofim wrote:
    KHarvey wrote: My only issue with longer variables is it takes me longer to type or I have to copy and paste. Once again, I normally don't use an IDE as I am not always working on the same computer.

    I don't use an IDE for Python, and I develop on about four different machines. A decent text editor will give you basic autocompletion (for symbols in the same file). Besides you should be optimizing for reading no writing -- if takes longer to type but makes the code a bit clearer, it's a good trade off.

    I guess I need to find better text editors then. :P

    setrofim wrote:
    KHarvey wrote:
    setrofim wrote:Actually, this is a good example of Hungarian notation being use as crutch to deal with bad code. Good practice in this case is to factor the code that does the SQL query (or code that adds stuff to HTML) into a separate function that takes the value to be added and the database query/HTML document it is to be added to as inputs. You then assume that all values passed into that function are unsafe and sanitize them within the function just before executing the query or adding the value to the HTML. You then don't need to worry about safe/unsafe distinction anywhere else in your code. In fact, Python's DB APIs do exactly that -- you pass the query you want to run and the values you want to run it with as separate arguments to the query() method of the DB connection. The query() method then sanitizes the values for you.

    I actually disagree with you here. This may not be very applicable to Python, or rather I have not had to write a piece of code that requires me to sanitize it yet. But in web programming, if a user submits a form you should check all the user submitted variables. For example if you have a drop down menu, very that the value submitted for the drop down menu is actually an option in said drop down menu. I usually do this verification at the top of my code rather than the bottom where I do my SQL updates or insertions. The main reason is there isn't any point to me processing the rest of the page for an XSS attack, especially if it is a long running script. That is normally where I assign the Clean variable at the top of my code when I do the verification. But I now see your point of rather than saying us or s or cln, I should just name the variable clean_username.

    Web form validation is deferent; here you should be validating both on the client side and on the server side. This does not invalidate my point, however. The important thing is to validate consistently -- either right as you get the input or just before submitting a query. And in both cases, sanitization is best outsourced to a dedicated function. Doing that, you don't need to worry about sanitization status of your variables in the rest of your code -- either they're never sanitized (in which case you sanitize before submitting to the database, or they are always sanitized (in which case you must make sure that is the case by carefully sanitizing on each user input). In Python, it makes sense to sanitize before running DB query, because then you only have to do it in one place; in other situations it might make sense to sanitize immediately as you get input form the user. But it is always either one or the other; and in both cases, the sanitization status of your variables is unambiguous.

    I agree, I have sanitation functions in each language that I code in. It's actually one of the first functions that I write when I pick up another language. There are some instances where you want to keep an unsanitized variable to use, but sanitize it before submitting to a DB. That mostly occurs with web and HTML encoding. In those cases, having something that says clean_username or dirty_username makes it easier to identify. I would use this more as a double check (as I do now) than a hard set rule in my code. But as you said it all is based off where the sanitation happens, if it is inline then changing the name of the variable is moot. But if it is out line then having that name change I think would be a good thing.

    setrofim wrote:
    KHarvey wrote:In school we were taught one language (for the most part), but once I hit the real world and the industry I found the programmers that could only use one language were lacking (I've got a couple of programmers that I am dealing with now that are like that).

    For what it's worth, I regularly write Python, C#, C++ and R. If we were taking about implementing your code in C#, my suggestions on how to change it would be very different (though the underlying principles, such as writing in the language, my stance on Hungarian notation, etc, are obviously language agnostic).

    You code in more languages than I do. I am trying to use PHP and Python for as many things as I can. I know that invalidates many of my points as I should use the right language (tool) for the right job. My issue is, is that I am a Network Admin and not a programmer. Most of my code is scripts to help in administering the network or applications. The few applications that I have written for the company are done in the proper language for the job (PHP for web, and .NET for Outlook tools). I am porting a lot of my scripts from VB and VBScript to Python as we are deploying more and more *NIX boxes. So switching to a cross platform language makes sense, although some of my functions look a little funky as I test for OS before running.

    setrofim wrote:
    KHarvey wrote:The programmers that could pick up what ever they needed always advocated being able to build your code around the logic and not necessarily the language / syntax.

    That is very good advice, but it does not contradict my point. As an example, this is quicksort implemented in C. And these are quicksort implementations in Python (particularly, look at the last one). The code is completely different (port of Python code to C is not obvious). The underlying logic (the quicksort algorithm) is exactly the same and is not driven by either language.

    I follow what you are saying. And reading through those examples brings back flash backs of C :shudder:
    The logic is the same, well, they do the same thing so that makes sense. After reading the different ports a couple of times it does make sense how they are doing it. I will agree that Python is easier to read than C, but the more I look at them the more I see the same thing. Which once again they are the same thing :roll:
    I think we are saying similar things, you are just able to articulate it better than I. The part that set me down this path was the comment on .find("Hex-STRING) != -1 was unpythonic. Then with all the research and reading that I was doing it was coming down to "you must do it this way or it is wrong". I understand that there are different ways to handle things in each language, some are more efficient than others. My point is by doing that, it can limit your coding to a specific language. I believe there is a healthy medium.
    Okay I have lost my train of thought too many times comparing the code. So I will leave it at what I just said as I can't figure out a way to articulate what I am saying. :)
    Using the link examples that you gave, I would have probably used a while loop in Python like they did in C.
    KHarvey
     
    Posts: 34
    Joined: Tue Mar 19, 2013 5:13 pm
    Location: US

    Re: Inefficient Code Assistance

    Postby setrofim » Thu Mar 28, 2013 11:40 pm

    KHarvey wrote:Honestly "bp", "ii" don't really have a meaning. They stand for Bridge Port and ifIndex but those don't really have a meaning in the code, they are just kind of a place holder.

    In that case they probably should not be in the variable name. All you're doing is making the variable harder to type, harder to remember and (if you have a few following the same convention) harder to distinguish. If those parts don't add to understanding of the code, you're just making things harder for no benefit.

    KHarvey wrote: Six months from now, those variables will mean pretty much the exact same thing.

    Sure, but will you remember what they mean in six months? The code may be clear now. But in six months you would have written a ton more code, your conventions would have evolved, or you might have moved on to doing something completely different before coming back to this. You need to write the code that the future you (or potentially, some else that will be asked to maintain your code) will understand, not just code that the current you understands.

    KHarvey wrote:

    You should use update() when you want add key-value pairs from one dict to another. List's append() is a bit different as it only appends one value to the list, and that's what you should use to do that (the [] notation can only be used to update/change existing elements, not add new ones). A closer equivalent for a list would be extend() -- this extends one list with elements of another. Check out the docs they explain it more clearly and give some examples.

    KHarvey wrote:I don't quite understand the point to the single line of code that you are proposing. Doesn't that single line do a for loop as well, but it is just inline?

    Yeah, sort of; but not quite. Logically, it is doing the same thing as a for loop, but underneath it is a different language construct that just happens to reuse the for keyword (it's called a generator expression, and there are related constructs list and (in Python 3) dict comprehensions.). The point of using it is that you're describing the same operation in less code. The less code you write the better. Less code is easier to understand, and you can't have bugs if you don't have code.

    KHarvey wrote:The refactoring to the single line seems to go against what we have been discussing above and more back to the way of when I was doing a bunch of things on a single line.

    Yes you're right. This is somewhat contradictory to the previous example. That's why it's called software engineering :). It's all about the trade offs. One of your goals as a programmer is to reduce the cognitive load involved in understanding your code. Long sequences of operations on a single line are hard to understand; they should be split up. A large number of lines describing an operation are hard to understand; they should be chunked together if possible. Yes, there is some conflict between these two statements. Note however that by "chunking" can mean different things. You can turn several lines into one without creating convoluted one-liners by breaking them out into a function.

    There are other factors involved here. That construct is pretty common (you'll encounter it a lot when reading Python code), so the cognitive load involved in understanding it is fairly low. Also the previous line was of the form a().b().c(), where here the construct is more like c(b(a())); in the former, you need to trace through the entire line to see what is happening; in the latter, you see immediately that you're creating a dict and what follows are basically "implementation details" of how that dict gets created. Finally, the number of logical things that are happening here is still less than in the one liner from before.

    Note that generator expressions (and list/dict comprehensions may be used to create some horribly unreadable and convoluted one liners too (they are not a licence to stick as much on one line as possible). A couple of regulars on this forum used to have a lot of fun playing with those.

    KHarvey wrote:If I am able to grasp classes then I think a lot of the things that I do will be cleaned up by them, and my code should look much better.

    In indeed. Classes are all about encapsulation. The idea that rather than worrying about managing a bunch of variables in your code, you stick them in the class. You then write methods for the class to manipulate those variables. In the rest of the code, you then just have the one variable that contains the instance of the class and you tell it to do things (that translate to manipulating your original variables, but you no longer need to worry about that in the code -- it's "encapsulated").


    KHarvey wrote:I guess I need to find better text editors then. :P

    My personal recommendation would be Vim (and how are you a network admin and not already using vim or emacs?), but it comes with a learning curve; so a more accessible alternative might be Sublime Text 2.

    KHarvey wrote:My issue is, is that I am a Network Admin and not a programmer.

    If you write code that runs "in the wild", that others rely on and that needs to be maintained going forward, then you're a programmer, even if that's not what it says in your job title. Before, you said you were a "scripter" rather than a "programmer" -- it's the same thing. Code is code. Sure, there is a lot of difference between writing a web app vs a device driver vs a CRM system vs a script to monitor network traffic. But some things are universal. All code needs to work (first and foremost); all code needs to be robust (to a varying degree); all code needs to be (reasonably) performant; all code needs to be maintainable. And these are the things we're talking about here.

    Maintainability is especially import as code tends to live longer than anyone expects it to. You should always "code as if the person who ends up maintaining your code is a violent psychopath who knows where you live".
    setrofim
     
    Posts: 288
    Joined: Mon Mar 04, 2013 7:52 pm

    Re: Inefficient Code Assistance

    Postby KHarvey » Fri Mar 29, 2013 2:18 pm

    setrofim wrote:
    KHarvey wrote:Honestly "bp", "ii" don't really have a meaning. They stand for Bridge Port and ifIndex but those don't really have a meaning in the code, they are just kind of a place holder.

    In that case they probably should not be in the variable name. All you're doing is making the variable harder to type, harder to remember and (if you have a few following the same convention) harder to distinguish. If those parts don't add to understanding of the code, you're just making things harder for no benefit.

    The names aren't important, but the fact that they exist I think is.

    setrofim wrote:
    KHarvey wrote: Six months from now, those variables will mean pretty much the exact same thing.

    Sure, but will you remember what they mean in six months? The code may be clear now. But in six months you would have written a ton more code, your conventions would have evolved, or you might have moved on to doing something completely different before coming back to this. You need to write the code that the future you (or potentially, some else that will be asked to maintain your code) will understand, not just code that the current you understands.

    My point was that what they currently mean is place holder. So in 6 months from now they will still mean place holder. This code is something that I actually wrote in VBScript around 10 or 11 years ago :shock: . I think I'm starting to get old.
    Not much has changed in it, other than I have ported it to different languages through out the years as required by my position. I moved it to Python around 3 or 4 years ago, but I never really had a chance to clean it up. So this go around I am refactoring the code to be more efficient, and follow better coding practices. I have around 20'ish other scripts like this that compose my app that I will be going through. This is just the 4th one of many.
    The other part that I am fixing as at each company I deployed this at it has required heavy customization for it to run properly. I am now generalizing the scripts where they go through and find the info, rather than me supplying the info (minus an XML document that houses some information.

    setrofim wrote:
    KHarvey wrote:The refactoring to the single line seems to go against what we have been discussing above and more back to the way of when I was doing a bunch of things on a single line.

    Yes you're right. This is somewhat contradictory to the previous example. That's why it's called software engineering :). It's all about the trade offs. One of your goals as a programmer is to reduce the cognitive load involved in understanding your code. Long sequences of operations on a single line are hard to understand; they should be split up. A large number of lines describing an operation are hard to understand; they should be chunked together if possible. Yes, there is some conflict between these two statements. Note however that by "chunking" can mean different things. You can turn several lines into one without creating convoluted one-liners by breaking them out into a function.

    There are other factors involved here. That construct is pretty common (you'll encounter it a lot when reading Python code), so the cognitive load involved in understanding it is fairly low. Also the previous line was of the form a().b().c(), where here the construct is more like c(b(a())); in the former, you need to trace through the entire line to see what is happening; in the latter, you see immediately that you're creating a dict and what follows are basically "implementation details" of how that dict gets created. Finally, the number of logical things that are happening here is still less than in the one liner from before.

    Note that generator expressions (and list/dict comprehensions may be used to create some horribly unreadable and convoluted one liners too (they are not a licence to stick as much on one line as possible). A couple of regulars on this forum used to have a lot of fun playing with those.

    I figured as much. This will be something that will take me some time to get used to and use it properly.

    setrofim wrote:
    KHarvey wrote:If I am able to grasp classes then I think a lot of the things that I do will be cleaned up by them, and my code should look much better.

    In indeed. Classes are all about encapsulation. The idea that rather than worrying about managing a bunch of variables in your code, you stick them in the class. You then write methods for the class to manipulate those variables. In the rest of the code, you then just have the one variable that contains the instance of the class and you tell it to do things (that translate to manipulating your original variables, but you no longer need to worry about that in the code -- it's "encapsulated").

    After reading through the Python documentation again I think I understand a little bit better how to design classes (I think). But when I tried to design a class for this script I couldn't quite figure out how, or maybe why, I would use a class. Pretty much all I could figure out is that I would just be using the variables in a class rather than the variables that I am using now. I guess the point is to move this whole script into a class of it's own? Which currently doesn't have much benefit, that I can see or understand. Although in the future if I started placing my code in classes I could start building one script that calls the individual classes (currently scripts) so that I don't have 20'ish scripts that need to run.
    I will try playing with this some more.

    setrofim wrote:
    KHarvey wrote:I guess I need to find better text editors then. :P

    My personal recommendation would be Vim (and how are you a network admin and not already using vim or emacs?), but it comes with a learning curve; so a more accessible alternative might be Sublime Text 2.

    I prefer vim over emacs. The issue is, not every OS that I run has them installed by default, especially on some of my servers. So rather than just adding more apps to a server I would just run with vi since it is installed on pretty much everything but Windows by default. Not using IDE's or fancy text editors has it's downsides. But I also don't need to really install anything special on any computers or servers to start changing config files or programming. This was extremely handy when I was doing consulting as some companies had a mountain of paperwork that had to be filled out before an application could be installed on a server. Some of them just flat wouldn't allow their "pristine" image to be compromised.
    As for learning curves, I don't think there is another text editor with as big of a learning curve as vi.
    As far as network administration goes I am a minimalist as efficiency is paramount.

    setrofim wrote:
    KHarvey wrote:My issue is, is that I am a Network Admin and not a programmer.

    If you write code that runs "in the wild", that others rely on and that needs to be maintained going forward, then you're a programmer, even if that's not what it says in your job title. Before, you said you were a "scripter" rather than a "programmer" -- it's the same thing. Code is code. Sure, there is a lot of difference between writing a web app vs a device driver vs a CRM system vs a script to monitor network traffic. But some things are universal. All code needs to work (first and foremost); all code needs to be robust (to a varying degree); all code needs to be (reasonably) performant; all code needs to be maintainable. And these are the things we're talking about here.

    Maintainability is especially import as code tends to live longer than anyone expects it to. You should always "code as if the person who ends up maintaining your code is a violent psychopath who knows where you live".

    That's what my boss says too, that's why she keeps having me write more and more things. :roll:
    I also completely agree with the pyschopath quote, I've heard it many times. That was why I was always adding so many comments to my code. As I said, I haven't found my happy medium yet, and it will probably take a little longer than most. I normally only code things for a couple of weeks out of the year.


    On a different note, I have almost finished my refactoring of this script. I think I have 2 more problems that I need to fix before I post it though. Hopefully I can get that done today. I didn't really get a chance to work on it yesterday.
    Also before when I ran my script it took 45.2 minutes to complete. When I quit parsing the commands inline, the script run time dropped to 7.8 minutes. Then when I started to change my code to remove the multiple loops, and simplified it, the run time has dropped to around 4.1 minutes.
    I think I will be going back to the other scripts that I have completed, and dumping all the command outputs to a list or dictionary, and then process the results after the command has finished. In this case it dropped my run time by 83%. While my other scripts aren't as long, I doubt that I will have the same results, but I figure it should have some time off of them.
    KHarvey
     
    Posts: 34
    Joined: Tue Mar 19, 2013 5:13 pm
    Location: US

    Next

    Return to General Coding Help

    Who is online

    Users browsing this forum: No registered users and 8 guests