2
\$\begingroup\$

I have the following code:

resolved = []
rejected = []
for file in files_within("./_input/"):
    file = File(file)
    if file.extension.lower() == "gdb":
        resolved.append(file)
    else:
        rejected.append(file)
MAP["resolved"] = resolved
MAP["rejected"] = rejected

And i would like to improve it to avoid this part:

MAP["resolved"] = resolved
MAP["rejected"] = rejected

But without affecting the performance.

I was thinking about that:

MAP["resolved"] = []
MAP["rejected"] = []
for file in files_within("./_input/"):
    file = File(file)
    if file.extension.lower() == "gdb":
        MAP["resolved"].append(file)
    else:
        MAP["rejected"].append(file)

But i don't know if is a good practice.

Python version 2.6.

\$\endgroup\$
4
\$\begingroup\$

Each MAP["resolved"] has to perform a hash on "resolved", and then check up the value in the dictionary's internal structure. This is more costly then looking up a single variable.

Just like in the first, you can assign a local variable, resolved, but you can assign to the dictionary at the same time:

resolved = MAP["resolved"] = []

If you only mutate resolved, then you mutate MAP["resolved"] too. So you can't overwrite resolved with resolved = ['new list'], for example. Which can get you:

MAP["resolved"] = resolved = []
MAP["rejected"] = rejected = []
for file in files_within("./_input/"):
    file = File(file)
    if file.extension.lower() == "gdb":
        resolved.append(file)
    else:
        rejected.append(file)

I am also unsure on what your functions are, I don't remember them being in the standard library, and so I won't suggest any changes to them.

\$\endgroup\$
3
\$\begingroup\$

Assuming that:

  • you only have two possible results
  • you can't have two files with the same name

Then you only need to iterate once and then make the set difference between the results. Something like:

all_files = set([File(file) for file in files_within("./_input/")])
MAP["resolved"] = [file for file in all_files if file.extension.lower() == "gdb"]
MAP["rejected"] = list(all_files - set(MAP["resolved"]))

Note: I didn't test this, it's just the general idea.

EDIT: As pointed out, the original code stores File objects, not file paths.

\$\endgroup\$
  • \$\begingroup\$ The original code stores File objects; your solution stores the paths. \$\endgroup\$ – 200_success Jan 11 '17 at 19:32
  • \$\begingroup\$ Ops, you're right, I missed that the file variable was the same one being re-assigned. I'll edit it to see if I can fix it. \$\endgroup\$ – ChatterOne Jan 11 '17 at 20:05

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.