WTF: Case for a Join

 I found this code in our repository:

roles = ""

for i, id in enumerate(account_ids):

    if i > 0:

        roles = roles + ","

        if i == len(account_ids) - 1:

            roles = roles + "\"arn:aws:iam::*:role/" + entity_name + "-" + id + "-admin"

        else:

            roles = roles + "\"arn:aws:iam::*:role/" + entity_name + "-" + id + "-admin\""

    if i == 0:

        if i == len(account_ids) - 1:

            roles = roles + "arn:aws:iam::*:role/" + entity_name + "-" + id + "-admin"

        else:

            roles = roles + "arn:aws:iam::*:role/" + entity_name + "-" + id + "-admin\""

It takes some time to understand what is going on here, but basically, we are building a coma separated list of AWS roles from a list of account IDs.

When you see a condition based on the index of a for loop, you start to feel a very strong code smell.

First, the test for the index being bigger than zero, or equal to 0, is mainly made for handling the coma. But not only. There are then tests repeated, with almost similar codes, to check if we are on the last iteration, all this to discover if we have to add a " character at the beginning or the end of our string.

At the end, it produces a string in the form: role1","role2","role3

The reason that there is no quotation marks at the beginning or the end of the string, is that ultimately, it will be put in a template that is declared with the marks already there, in this form: "__ROLES__".

All this code is quite bad, and the coma and quotation marks handling can all be left to a call to the join function:

roles = '","'.join([f"arn:aws:iam::*:role/{entity_name}-{id}-admin"

    for id in account_ids])

From 13 lines to 1, I kind of like it.