Removed a mutable default param in Serializer.serialize(). #286

Merged
merged 1 commit into from Feb 22, 2014

Conversation

Projects
None yet
3 participants
Contributor

malcolmt commented Nov 14, 2011

I haven't seen this pop up explicitly as a bug in code; only noticed it
through code inspection.

There's a small chance somebody might stash something in the default provided "options" dictionary and it will be there for all future calls to serialize(), typically causing much confusion for the developer making the function call. A quick grep though the code does show empty dictionaries being used anywhere else as a default argument.

@malcolmt malcolmt Removed a mutable default param in Serializer.serialize().
I haven't seen this pop up explicitly as a bug in code; only noticed it
through code inspection.
fb7326d
Contributor

malcolmt commented Nov 15, 2011

Looking at this with fresh eyes this morning and seeing how you've done it in other places, it should probably be

options = options or {}

rather than "if options is None: options = {}", simply for consistency. I'll leave the pull request open as a reminder of the location, in preference to creating an entirely new branch (I know you cut-and-paste anyway).

Sorry for the confusion; didn't notice the half dozen other places you were using options=None when I was quickly working last night. My bad.

@toastdriven toastdriven added a commit that referenced this pull request Feb 22, 2014

@toastdriven toastdriven Merge pull request #286 from malcolmt/mutable-default-fix
Removed a mutable default param in Serializer.serialize().
78b8ca5

@toastdriven toastdriven merged commit 78b8ca5 into django-tastypie:master Feb 22, 2014

Contributor

toastdriven commented Feb 22, 2014

This should have been merged the minute you opened it. Now that it can't possibly improve things for you, applying it feels hollow. Thanks for everything, Malcolm, you are missed.

@kamikaze kamikaze commented on the diff Mar 26, 2014

tastypie/serializers.py
"""
Given some data and a format, calls the correct method to serialize
the data and returns the result.
"""
desired_format = None
+ if options is None:
@kamikaze

kamikaze Mar 26, 2014

if not options: ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment