0
\$\begingroup\$

I have such method where:

  • primary_images is dict,
  • additional is dict,
  • get_uri_text is some decorating method,
  • marker is telling how to parse two dicts.

How can I simplify this method?

    @staticmethod
    def decorate_sort_images(primary_images, additional_images, get_uri_text, marker=False):
        srt_keys = sorted(primary_images.keys())
        new_result = OrderedDict()
        if not additional_images:
            #only primary
            for _key in srt_keys:
                new_result[_key] = [get_uri_text(iter_el) for iter_el in primary_images[_key]]
            return new_result
        for _key in srt_keys:
            new_result[_key] = [get_uri_text(iter_el) for iter_el in primary_images[_key]]
            if marker:
                if _key in additional_images:
                    new_result[_key] += [get_uri_text(iter_el) for iter_el in additional_images[_key]]
            else:
                new_result[_key] += [get_uri_text(iter_el) for iter_el in additional_images[_key]]
        return new_result
\$\endgroup\$

closed as unclear what you're asking by Toby Speight, Ludisposed, Graipher, Zeta, Nic Hartley Dec 11 '18 at 1:32

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 3
    \$\begingroup\$ What do the dictionaries contain? What is the method supposed to be doing? \$\endgroup\$ – jonrsharpe Feb 3 '15 at 8:09
  • 2
    \$\begingroup\$ The current question title, which states your concerns about the code, is too general to be useful here. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review: Asking Questions for guidance on writing good question titles. \$\endgroup\$ – Toby Speight Dec 7 '18 at 8:29
2
\$\begingroup\$

As far as I can tell, the difference between the conditional branches is only whether or not to avoid KeyError occurring at additional_images[_key] when key does not exist. This code should have the same behavior:

@staticmethod
def decorate_sort_images(primary_images, additional_images, get_uri_text, marker=False):
    srt_keys = sorted(primary_images)
    new_result = OrderedDict()
    allow_key_error = additional_images and not marker
    for _key in srt_keys:
        new_result[_key] = [get_uri_text(iter_el) for iter_el in primary_images[_key]]
        if allow_key_error or _key in additional_images:
            new_result[_key] += [get_uri_text(iter_el) for iter_el in additional_images[_key]]
    return new_result        

When additional_images is empty, _key in additional_images is always False, so we don't need the first if statement any more.

Note that I also removed a redundant .keys() from sorted(primary_images).

\$\endgroup\$

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