In Defense of Copy and Paste

Re:In Defense of Copy and Paste by danso on Friday February 08, 2013 @01:44PM (#5189414)

OK, I'm obviously missing something, and part of the problem is that I'm not a Python programmer so my brain is obviously in "skim-mode".

Couldn't the problematic DRY pattern be alleviated by refactoring the following call:

    filter_profanity = kwargs.pop('filter_profanity')
    tweets = Tweet.objects.filter(**kwargs)
    if filter_profanity:
        tweets = itertools.ifilter(lambda t: not t.is_profane(), tweets)
    return render(request, template, {'tweets': tweets})

Into something like:

    def tweet_list(request, **kwargs)
       ...

       tweets = get_filtered_tweets(kwargs)
       ...

    def get_filtered_tweets(**args)
       filter_profanity = args.pop('filter_profanity')
       if filter_profanity 
          etc....
        end
       return tweets
    end

Why does the logic for the Tweet filtering have to be encapsulated in the rendering function?

// edit:

What might help is if the OP showed how the non-refactored code would look with the profanity_filter and pagination features. I agree that his refactored proposal is confusing...I'm just having a hard time imagining how the non-refactored version would be less so.

[ Parent | on HN ]

Re:In Defense of Copy and Paste by zacharyvoase on Friday February 08, 2013 @01:50PM (#5189445)

That still seems complicated to me. You're still passing around an object representing a collection of arguments/options, and there are functions whose outputs vary based on this object's state—it's not immediately inferable from the call to `get_filtered_tweets` whether the result will be filtered for profanity or not. So you might as well have that bit of code right next to the code which uses its result, rather than splitting it up into two separate (but still complex) parts.

EDIT: I added a bit here on how I would do it better without needless refactoring: http://localhost:3000/2013/02/08/copypasta/#a-better-solutio...

[ Parent | on HN ]

Re:In Defense of Copy and Paste by drostie on Friday February 08, 2013 @02:05PM (#5189529)

I believe they intended the 'proper' code to look like:

    def global_feed(request):
        tweets = Tweet.objects.all()
        tweets = itertools.ifilter(lambda t: not t.is_profane(), tweets)
        return render(request, 'global_feed.html', {'tweets': tweets})

    tweets_per_page = 20
    def user_timeline(request, username):
        tweets = Tweet.objects.filter(user__username=username)
        page = request.GET.get('page', 1)
        offset = (page - 1) * tweets_per_page
        tweets = tweets[offset:offset + tweets_per_page]
        return render(request, 'user_timeline.html', {'tweets': tweets})

[ Parent | on HN ]

Re:In Defense of Copy and Paste by danso on Friday February 08, 2013 @02:27PM (#5189636)

So the problem with that is seems to be...what if there was another view that required pagination?

So the two unpleasant scenarios seem to be this:

1) The OP's assertion that DRYing the code may unintentionally break functionality in all the places that use it.

2) The DRY assertion: copy-pasting functionality, such as pagination, makes it more likely that the pagination functionality won't be properly updated across all the modules that use it.

I guess it's a case of YMMV...because in this hypothetical app, it doesn't seem likely that the number of views will multiply, thus making it easier to update the copy-pasted code. But that seems like a mindset as prone to future problems than one that is more DRY-minded.

[ Parent | on HN ]