Trying to clean up my code with helper function instead of using repeated code-CodePudding

The following code works, and does exactly what it should, I submitted it for review by my boss, and he liked it but thinks that it could be cleaner, and suggested I should wrap this code in a helper function and then pass in the different parameters. Being relatively new to programming and python, I understand OOP, but I am a little confused on this objective. Here is the code:

For Reference (This is just fake example data, ignore the phonenumbers and emails lol):

notify_users = [
{'Name': 'Timmy Turner', 'Number':'3245234523', 'Email':'[email protected]','Days': 15},    
{'Name': 'Jimmy Fallon', 'Number':'3245234523', 'Email':'[email protected]','Days': 30}
]
expired_users = [
{'Name': 'James Issac', 'Number':'3245234523', 'Email':'[email protected]','Days': -4},    
{'Name': 'Roger Lewis', 'Number':'3245234523', 'Email':'[email protected]','Days': -5}
]

msg = "Hello %s, %s. \n \n***To change your password, press CTRL   ALT   DEL and select change password***.\n\n\n- MY COMPANY IT"

def send_notifications():
            # Send SMS
            client = Client(twilio_account_sid, twilio_auth_token)
            for d in notify_users:
                message = client.messages.create(
                    to=d['Number'],
                    from_=twilio_from_phone_number,
                    body=msg % (d['Name'], f"you have {d['Days']} days left to reset your password."))
                print(message.sid)
                # Send emails
                requests.post(
                    self.app.config.get("https://api.mailgun.net/v3/MYDOMAIN/messages"),
                    self.app.config.get(auth=("api", mailgun_api)),
                    self.app.config.get(data={"from": "IT <mailgun@MYDOMAIN>",
                                              "to": d['Email'],
                                              "subject": "Password Reminder",
                                              "text": msg % (d['Name'], f"you have {d['Days']} days left to reset your password.")}))
            # Send notifications for expired users
            # Send SMS
            for d in expired_users:
                message = client.messages.create(
                    to=d['Number'],
                    from_=twilio_from_phone_number,
                    body=msg % (d['Name'], "your password has expired, please change it asap to avoid any issues that could occur with your account"))
                print(message.sid)
                # Send expired emails
                requests.post(
                    self.app.config.get("https://api.mailgun.net/v3/MYDOMAIN/messages"),
                    self.app.config.get(auth=("api", mailgun_api)),
                    self.app.config.get(data={"from": "IT <mailgun@MYDOMAIN>",
                                              "to": d['Email'],
                                              "subject": "Password Reminder",
                                              "text": msg % (d['Name'], "your password has expired, please change it asap to avoid any issues that could occur with your account")}))

        # Determine if lists contain users
        send_notifications()

I completely understand where he is coming from, because I am essentially repeating the same code twice. What I was thinking was some sort of helper function that would wrap this code, so I can just pass in parameters def notify(user_list, msg_template) but I am unsure how to really implement this.

This is basically what I want to accomplish, 2 calls to do the logic for expired and notify users:

notify(notify_users, notify_msg)
notify(expired_users, expired_msg)

If anyone can provide some tips or insight on how I could go about this, I would greatly appreciate it. Thanks!

CodePudding user response：

I think this is roughly what you want -- since there's no MRE I can't actually run this code to verify that it works. All you really need to do for this type of refactor is take the code that you've copied and pasted, and make a single version of it where the part of it that changes is a variable.

def send_notifications():
    client = Client(twilio_account_sid, twilio_auth_token)

    def notify(user_list, msg_template):
        for d in user_list:
            message = client.messages.create(
                to=d['Number'],
                from_=twilio_from_phone_number,
                body=msg % (d['Name'], msg_template.format(**d))
            )
            print(message.sid)
            # Send emails
            requests.post(
                self.app.config.get("https://api.mailgun.net/v3/MYDOMAIN/messages"),
                self.app.config.get(auth=("api", mailgun_api)),
                self.app.config.get(data={
                    "from": "IT <mailgun@MYDOMAIN>",
                    "to": d['Email'],
                    "subject": "Password Reminder",
                    "text": msg % (d['Name'], msg_template.format(**d))
                })
            )

    # Send warnings for users who are about to expire
    notify(notify_users, "you have {Days} days left to reset your password.")

    # Send notifications for expired users
    notify(expired_users, "your password has expired, please change it asap to avoid any issues that could occur with your account")