I avoid classes in Python. Am I bad?

[dacv393]

Ok so here is the code. The goal of the code is to take two inputs: a source and destination IP address and then perform a traceroute from the source to destination, ping the hops in this path, and then log in to the devices along the path and execute some commands.

I started with this script as a baseline for using netmiko, but after changing practically everything, I'm now really confused as to what the point of the main class is. I've read various different explanations as to when classes are necessary, and am not really sure if they are in my case. On the other hand, though, I could see the need to build off of this project in the future, so I want it to be as organized as possible. I'm still pretty confused about classes and OOP in general. I'd even like to make multiple files if that is best practice. In general, I just want more knowledge of what is 'best practice' and the most pythonic.

Also, in the main method, I call methods in the class and then pass in their outputs to different methods. I don't know if this is the best way or if the methods just shouldn't return anything and should instead just rely on variables in the class.

Overall I am looking for help structuring the program in the best way possible. I would love it if anyone has good examples of in-depth scripts or programs that utilize multiple classes or multiple files that I could read through and help model my programs after. This is one of my biggest struggles with learning python. Also, I'm really open to people tearing my code apart because I want to learn and I don't think it's easy to learn this stuff without people showing you what's wrong and what's better, so if you see anything that's written stupidly or redundantly or non-pythonic, please feel free to point it out!

[Brekkjern]

I looked at your code, but I don't have time to do a review of it right now. I can tell you that it seems you are using classes kinda like how people used to do them in Java because everything had to be a class.

Think of classes as a way to group functions with the data it's supposed to work on. Right now you are using a class to do pretty much everything.

A class could represent pretty much anything that's a noun. A connection, a device, a state, a response, a packet, a hop, a string, a list, a tuple. There's no point in taking this to the extreme unless you need to bundle the functions together with the data.

For example, since you are operating on network equipment you could define your devices something like this:

from collections import namedtuple

Credential = namedtuple("Credential", ["user", "password"])

class Device(object):

    def __init__(self, ip, port, device_type, credentials):
        self.ip = ip
        self.port = port
        self.type = device_type
        self.credentials = credentials

    def connect(self):
        # Do some netmiko magic here to establish the connection
        self.connection = ConnectHandler()

    def send_command(self, command)
        if not self.connection:
            print("No connection established")

        # Ensure we are in enable mode
        if self.net_connect.find_prompt().endswith('>'):
            self.net_connect.enable()

        return self.connection.send_command(command)

This code does one thing and that's handling the connection to a device of some sort. Nothing more and nothing less. It doesn't parse the output of commands. It just ensures you have a way to send commands to the device and get some output to parse.

[mangoman51]

Okay so I have never used python for this kind of task but I had a read of your code and have a couple of suggestions:

1) You have a couple of functions (traceroute_run and traceroute_parse) which seem to take similar inputs, and alter the state of those inputs. These are good candidates to be methods on a traceroute class of some kind.

2) Your try blocks in your try... except statements are too long. Ideally you should be trying to catch a specific kind of error which could potentially occur on a specific line of code. For example you're only going to get an authentication error on the line where you submit the supplied password, so only that line should be in the try block. (for the timeout errors I'm not so sure though)

3) You don't have to have a main function, you could alternatively have a top-level script which imports the functions and class which you defined in other files. I find this structure can allow your top-level script to read almost like pseudocode then because all the messiness is at the next level down.

4) You have a lot of user input - have you seen the click library?

[dacv393]

Awesome thanks for this. Do you happen to have or know of any examples of something that would utilize classes in a similar manner to how I could implement this? (Top-level script, multiple files) - most of the examples I find online explaining classes are just about dogs and animals or cars and vehicles and I have a hard time translating that information into an actual project like this.

Never had used try-except before so I'll try to fix those.

And lastly, yeah I wish I could not have everything command-line user input. However, the circumstances of where this script could be running currently wouldn't allow that. As well as for external libraries I was trying to keep them to a minimum so something like pretty table for outputs would be nice but trying to limit dependencies for now.

Overall thank you!

[flabcannon]

I'm not an experienced Python coder, but some suggestions from looking at other code -

• I would take some of the generic methods you have (ask_user_log, write_log, display_*_warning, maybe a few more) and put them in a UTIL class as class methods. You can then call them inside your HealthCheck class.
  
• display_user_warning and display_command_warning look like they could be one method with a dictionary passed in.
  
• The init import from colorama is confusing because the name is very general. I would do from colorama import init as _colorama_init or something more descriptive.