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?


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)
| improve this answer | |
  • \$\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 '17 at 14:36
  • \$\begingroup\$ @Snowbody Thanks for pointing out about keywords. Why does it matter if those are constants? \$\endgroup\$ – Solomon Ucko Dec 7 '17 at 16:55
  • \$\begingroup\$ Because then they can be passed in and used directly, saving a call to getattr() \$\endgroup\$ – Snowbody Dec 7 '17 at 18:15
  • \$\begingroup\$ @Snowbody That's what I was wondering. I'll change that. \$\endgroup\$ – Solomon Ucko Dec 7 '17 at 20:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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