Python Forum
Thread Rating:
  • 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Duplicate functions
#1
Hi,

So I am writing a small Python app to carry out a few tasks and print the findings to the screen using the @Click module. Although it works as expected I have a lot of duplication in my functions:

role_details = []

    get_role = ec2.describe_instances(
        Filters=[
            {
                'Name': 'tag:role',
                'Values': [role]
            },])
    for reservation in get_role['Reservations']:
        for instance in reservation['Instances']:
            role_info = {}
            role_info.update({'InstanceID':instance['InstanceId']})
            role_info.update({'State':instance['State']['Name']})
            role_info.update({'PrivateIpAddress':instance['PrivateIpAddress']})
            role_info.update({'InstanceType':instance['InstanceType']})
            for tag in instance['Tags']:
                if tag['Key'] in 'Name':
                    role_info.update({'Name':tag['Value']})
            for tag in instance['Tags']:
                if tag['Key'] in 'environment':
                    role_info.update({'Environment':tag['Value']}) 
                else:
                    role_info.setdefault('Environment','MISSING')
            for tag in instance['Tags']:
                if tag['Key'] in 'role':
                    role_info.update({'Role':tag['Value']}) 
                else:
                    role_info.setdefault('Role','MISSING')           
            role_details.append(role_info)
    return role_details
Where the Filters(Name/Value) differ between functions. I wondered whether there is a way to achieve the same results but consolidate into a single function where the Filters are the only changeable values?
Reply
#2
You talk about duplicate functions and then you post code that does not contain any functions. What's with that? I would expect at least two of the functions instead of one incomplete function as your example.

If you have a bunch of functions and the only way they differ is in some variable, pass the variable as an argument to the function.
Reply
#3
You have a very valid point and apologies for not providing the correct information. Please find below:

def get_instance_info(env):
       
    ec2_details = []
    
    get_ec2 = ec2.describe_instances(
        Filters=[
                {
                'Name': 'tag:environment',
                'Values': [env]
                }, ] )
    
    for reservation in get_ec2['Reservations']:
        for instance in reservation['Instances']:
            ec2_info = {}
            ec2_info.update({'InstanceID':instance['InstanceId']})
            ec2_info.update({'State':instance['State']['Name']})
            ec2_info.update({'PrivateIpAddress':instance['PrivateIpAddress']})
            ec2_info.update({'InstanceType':instance['InstanceType']})
            for tag in instance['Tags']:
                if tag['Key'] in 'Name':
                    ec2_info.update({'Name':tag['Value']})
            for tag in instance['Tags']:
                if tag['Key'] in 'environment':
                    ec2_info.update({'Environment':tag['Value']})
                else:
                    ec2_info.setdefault('Environment','MISSING')
            for tag in instance['Tags']:
                if tag['Key'] in 'role':
                    ec2_info.update({'Role':tag['Value']}) 
                else:
                    ec2_info.setdefault('Role','MISSING')             
            ec2_details.append(ec2_info)   
    return ec2_details
    
def get_ip_info(ipaddr):

    ip_details = []

    get_ip = ec2.describe_instances(
        Filters=[
            {
                'Name': 'private-ip-address',
                'Values': [ipaddr]
            },])
    for reservation in get_ip['Reservations']:
        for instance in reservation['Instances']:
            ip_info = {}
            ip_info.update({'InstanceID':instance['InstanceId']})
            ip_info.update({'State':instance['State']['Name']})
            ip_info.update({'PrivateIpAddress':instance['PrivateIpAddress']})
            ip_info.update({'InstanceType':instance['InstanceType']})
            for tag in instance['Tags']:
                if tag['Key'] in 'Name':
                    ip_info.update({'Name':tag['Value']})
            for tag in instance['Tags']:
                if tag['Key'] in 'environment':
                    ip_info.update({'Environment':tag['Value']}) 
                else:
                    ip_info.setdefault('Environment','MISSING')
            for tag in instance['Tags']:
                if tag['Key'] in 'role':
                    ip_info.update({'Role':tag['Value']}) 
                else:
                    ip_info.setdefault('Role','MISSING')            
            ip_details.append(ip_info)
    return ip_details

def get_role_info(role):

    role_details = []

    get_role = ec2.describe_instances(
        Filters=[
            {
                'Name': 'tag:role',
                'Values': [role]
            },])
    for reservation in get_role['Reservations']:
        for instance in reservation['Instances']:
            role_info = {}
            role_info.update({'InstanceID':instance['InstanceId']})
            role_info.update({'State':instance['State']['Name']})
            role_info.update({'PrivateIpAddress':instance['PrivateIpAddress']})
            role_info.update({'InstanceType':instance['InstanceType']})
            for tag in instance['Tags']:
                if tag['Key'] in 'Name':
                    role_info.update({'Name':tag['Value']})
            for tag in instance['Tags']:
                if tag['Key'] in 'environment':
                    role_info.update({'Environment':tag['Value']}) 
                else:
                    role_info.setdefault('Environment','MISSING')
            for tag in instance['Tags']:
                if tag['Key'] in 'role':
                    role_info.update({'Role':tag['Value']}) 
                else:
                    role_info.setdefault('Role','MISSING')           
            role_details.append(role_info)
    return role_details
Reply
#4
Something like this?
def item_details(items):
    details = []
    for reservation in items['Reservations']:
        for instance in reservation['Instances']:
            item_info = {}
            item_info.update({'InstanceID':instance['InstanceId']})
            item_info.update({'State':instance['State']['Name']})
            item_info.update({'PrivateIpAddress':instance['PrivateIpAddress']})
            item_info.update({'InstanceType':instance['InstanceType']})
            for tag in instance['Tags']:
                if tag['Key'] in 'Name':
                    item_info.update({'Name':tag['Value']})
            for tag in instance['Tags']:
                if tag['Key'] in 'environment':
                    item_info.update({'Environment':tag['Value']})
                else:
                    ec2_info.setdefault('Environment','MISSING')
            for tag in instance['Tags']:
                if tag['Key'] in 'role':
                    item_info.update({'Role':tag['Value']}) 
                else:
                    item_info.setdefault('Role','MISSING')             
            details.append(item_info)   
    return details

items = ec2.describe_instances(
    Filters=[{'Name': 'tag:environment', 'Values': [???]}, ] )
instance_details = item_details(items)

items = ec2.describe_instances(
    Filters=[{'Name': 'private-ip-address', 'Values': [???]},])
ip_details = item_details(items)
Or maybe this:
def item_details(name, value):
    items = ec2.describe_instances(
        Filters=[{'Name': name, 'Values': [value]}, ])

    details = []
    for reservation in items['Reservations']:
        for instance in reservation['Instances']:
            item_info = {}
            item_info.update({'InstanceID':instance['InstanceId']})
            item_info.update({'State':instance['State']['Name']})
            item_info.update({'PrivateIpAddress':instance['PrivateIpAddress']})
            item_info.update({'InstanceType':instance['InstanceType']})
            for tag in instance['Tags']:
                if tag['Key'] in 'Name':
                    item_info.update({'Name':tag['Value']})
            for tag in instance['Tags']:
                if tag['Key'] in 'environment':
                    item_info.update({'Environment':tag['Value']})
                else:
                    ec2_info.setdefault('Environment','MISSING')
            for tag in instance['Tags']:
                if tag['Key'] in 'role':
                    item_info.update({'Role':tag['Value']}) 
                else:
                    item_info.setdefault('Role','MISSING')             
            details.append(item_info)   
    return details

instance_details = item_details('tag:environment', ???)

ip_details = item_details('private-ip-address', ???)
Which one works better depends on how much flexibility you need it selecting the items of interest.

Look for the similarities and mark the block of repeated code. Now look at each difference to determine if it has to be different. In your code there ec2_details, ip_details and role_details play the same part and can have the same name (you know this because you changed these names when you were cutting and pasting your code). Change these to all be the same for each copy. Now the only thing that should be different between copies is the code that makes each function unique. Look for a way that you can remove this code or make it more general. In my first example I removed the code, in the second I made it more general.
Reply
#5
I'll take a look at both your suggestions and see which one fits best with the @click.option I am using.
Reply


Forum Jump:

User Panel Messages

Announcements
Announcement #1 8/1/2020
Announcement #2 8/2/2020
Announcement #3 8/6/2020