I have these 2 functions that are almost the same:

def handle_keywords(self, scan_id):
    keywords = Keyword.objects.all()
    for kwrd in keywords:
        self.process_rss_url(kwrd.rss, SourceType.GOOGLE_ALERTS, kwrd.id, kwrd.last_scan_time, scan_id)
        kwrd.last_scan_time = timezone.now()

def handle_rss(self, scan_id):
    all_rss = RSS.objects.all()
    for rss in all_rss:
        self.process_rss_url(rss.feed_url, SourceType.RSS, rss.id, rss.last_scan_time, scan_id)
        rss.last_scan_time = timezone.now()

What is a good way to reduce the code duplication?


1 Answer 1


Try this:

def generate_func(url_attr, type_class, source_type):
    def f(self, scan_id):
        for keyword in type_class.objects.all():
            self.process_rss_url(getattr(keyword, url_attr), source_type, keyword.id, keyword.last_scan_time, scan_id)
            keyword.last_scan_time = timezone.now()

    return f

handle_keywords = generate_func('rss', Keyword, SourceType.GOOGLE_ALERTS)
handle_rss = generate_func('feed_url', RSS, SourceType.RSS)
  • \$\begingroup\$ The "keywords" temp is unnecessary. I think SourceType.RSS and SourceType.GOOGLE_ALERTS are constants, but it's not certain from the code given. \$\endgroup\$
    – Snowbody
    Dec 7, 2017 at 14:36
  • \$\begingroup\$ @Snowbody Thanks for pointing out about keywords. Why does it matter if those are constants? \$\endgroup\$ Dec 7, 2017 at 16:55
  • \$\begingroup\$ Because then they can be passed in and used directly, saving a call to getattr() \$\endgroup\$
    – Snowbody
    Dec 7, 2017 at 18:15
  • \$\begingroup\$ @Snowbody That's what I was wondering. I'll change that. \$\endgroup\$ Dec 7, 2017 at 20:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.