Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SIM121] Use ".get" instead of "if X in dict: dict[X]" #50

Closed
MartinThoma opened this issue Mar 23, 2021 · 8 comments
Closed

[SIM121] Use ".get" instead of "if X in dict: dict[X]" #50

MartinThoma opened this issue Mar 23, 2021 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@MartinThoma
Copy link
Owner

MartinThoma commented Mar 23, 2021

Explanation

The get(key, default_fallback=None) method of a dictionary is often overlooked. It provides a clear interface that every Python developer should know.

Example

# Bad
name = "some_default"
if "some_key" in some_dict:
    name = some_dict["some_key"]

# Good
name = some_dict.get("some_key", "some_default")
@MartinThoma
Copy link
Owner Author

$ astpretty --no-show-offsets /dev/stdin <<< `cat example.txt`
Module(
    body=[
        Assign(
            targets=[Name(id='name', ctx=Store())],
            value=Constant(value='some_default', kind=None),
            type_comment=None,
        ),
        If(
            test=Compare(
                left=Constant(value='some_key', kind=None),
                ops=[In()],
                comparators=[Name(id='some_dict', ctx=Load())],
            ),
            body=[
                Assign(
                    targets=[Name(id='name', ctx=Store())],
                    value=Subscript(
                        value=Name(id='some_dict', ctx=Load()),
                        slice=Constant(value='some_key', kind=None),
                        ctx=Load(),
                    ),
                    type_comment=None,
                ),
            ],
            orelse=[],
        ),
    ],
    type_ignores=[],
)

@MartinThoma
Copy link
Owner Author

Related:

if "key" in some_dict:
    for item in some_dict["key"]:

@MartinThoma
Copy link
Owner Author

MartinThoma commented Mar 27, 2021

False-Positive (solved):

if key in some_dict:
    file = some_dict[key]
else:
    ...  # anything

@MartinThoma
Copy link
Owner Author

False-positive:

if key in some_dict:
    # other_dict["other_key"] does not exist otherwise!
    other_dict["other_key"] = some_dict['key']

@MartinThoma MartinThoma changed the title [New Rule] Use ".get" instead of "if X in dict: dict[X]" [SIM121] Use ".get" instead of "if X in dict: dict[X]" Mar 28, 2021
@Skylion007
Copy link
Contributor

I've also seen:

name = "some_default" if "some_key" not in some_dict else some_dict["some_key"]

@ROpdebee
Copy link

ROpdebee commented Aug 5, 2021

Kind of related, but might suit a separate rule:

# bad
if key in some_dict:
  some_dict[key].append(value)
else:
  some_dict[key] = [value]

# good
from collections import defaultdict
some_dict = defaultdict(list)
some_dict[key].append(value)

+ sets, dicts as values

@MartinThoma
Copy link
Owner Author

MartinThoma commented Mar 28, 2022

I've used defaultdict in the past, but in the past 1-2 years I've rather used this pattern:

# bad
if key in some_dict:
  some_dict[key].append(value)
else:
  some_dict[key] = [value]

# Good
if key not in some_dict:
  some_dict[key] = []
some_dict[key].append(value)

I think it's typically clearer than a default dictionary, especially if the dictionary is passed to other functions.

@MartinThoma
Copy link
Owner Author

See #52

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants