3
\$\begingroup\$

The code below works fine but seems really hard coded and I feel like I over-complicated the way it should be, I have a list containing hex and non-hex colors.

example : ['#333333', '#ff034a', 'red']

Basically, hex colors are converted to RGB, the nouns are compared in the database and returns a list of hex if the noun matches a query.

Here's the Output I want to keep (could be a numpy output too) :

[[(38, 30, 38)], [(245, 245, 245)], [(255, 51, 51), (255, 153, 153), (217, 38, 38), (242, 13, 13), (255, 62, 62)]]

Here's the hard coded junk I want to keep short and make faster :

list_of_colors = ['#333333', '#ff034a', 'red'] #can contain from 1 to 3 values

def orderDimentionalRGB(list_of_colors):
    dominant = []
    secondary = []
    complementary = []

    for index, color in enumerate(list_of_colors): 
        if Color.objects.filter(color=color).exists(): #check for noun
            colors = Color.objects.filter(color=color)
            lst_colors = numpy.array(colors.first().color_list.replace(',', '').split())

            for lc in lst_colors:
                rgb = hex_to_rgb(lc) #returns from hex string -> to tuple rgb
                if index == 0:
                    dominant.append(rgb)
                elif index == 1:
                    secondary.append(rgb)
                else:
                    complementary.append(rgb)

        else: #check for rgb
            color = hex_to_rgb(color)

            if index == 0:
                dominant.append(color)
            elif index == 1:
                secondary.append(color)
            elif index == 2:
                complementary.append(color)

    colors = [dominant, secondary, complementary]
    set_colors = [x for x in colors if x]

    return set_colors

Maybe using that much if, elif, else and array variables becomes redundant and makes things slower, how can I improve this code ?

\$\endgroup\$
  • \$\begingroup\$ I think this code needs more context, where does Color come from? When and how is this code used? \$\endgroup\$ – Ludisposed Sep 1 '17 at 12:50
  • \$\begingroup\$ @Ludisposed Color comes from a Django Model I don't think it's useful to understand django itself for this problem, just keep in mind that this will return a list of HEX. The final output will be used in a precis order like this [dominant, secondary, complementary] \$\endgroup\$ – Hiroyuki Nuri Sep 1 '17 at 13:36
2
\$\begingroup\$

As far as performance goes, you definitely need to understand your bottlenecks - profile the code properly and try to see what takes the most time - querying the database, converting hex to rgb or something else.

Here are few observations though:

  • don't do exists() and then filter() - just do filter() right away - it will return an empty queryset if no results
  • I am not sure if there is much sense in making a numpy.array - see if removing it would make a positive impact on performance
  • you don't need to keep track of indexes and then append to different lists based on an index - what if you would just collect the current results and then append to the resulting list at the end (see what I mean in the code below)
  • if a color starts with # you probably don't need to query the database at all

At the end, you may get something along these lines:

def orderDimentionalRGB(list_of_colors):
    results = []

    for color in list_of_colors:
        current_result = []

        if color.startswith("#"):
            current_result.append(hex_to_rgb(color))
        else:
            db_result = Color.objects.filter(color=color)
            if db_result:
                db_colors = db_result.first().color_list.replace(',', '').split()
                current_result += [hex_to_rgb(db_color) for db_color in db_colors]

        if current_result:
            results.append(current_result)

    return results
\$\endgroup\$

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.