# Replace range with enumerate

**URL:** <https://forum.ansible.com/t/replace-range-with-enumerate/19997>\
**Category:** Ansible Developer\
**Created:** [December 13, 2015, 7:06am UTC](https://forum.ansible.com/t/replace-range-with-enumerate/19997 "2015-12-13T07:06:52Z")\
**Posts on this page:** 2\
**Page:** 1

<div class="post-metadata">

**Author:** ![Usman\_Ehtesham](https://avatars.discourse-cdn.com/v4/letter/u/8e8cbc/32.png) [@Usman\_Ehtesham](https://forum.ansible.com/u/Usman_Ehtesham)\
**Post date:** [December 13, 2015, 7:06am UTC](https://forum.ansible.com/t/replace-range-with-enumerate/19997/1 "2015-12-13T07:06:52Z")

</div>

In the link [https://github.com/ansible/ansible/blob/devel/lib/ansible/cli/\_\_init\_\_.py#L84](https://github.com/ansible/ansible/blob/devel/lib/ansible/cli/ __init__.py#L84),

for i in range(0, len(self.arg)):

is bit redundant, when you can just do range(len(self.arg))

But its better (and more Pythonic) to use enumerate:

```auto
for index, item in enumerate(self.args):
    arg = item
    if arg in self.VALID_ACTIONS:
        self.action = arg
        del self.args[index]
        break

```

```auto

```

```auto
Also, on another note, consider using for-else to replace:

```

```auto

```

---

<div class="post-metadata">

**Author:** ![Toshio\_Kuratomi](https://avatars.discourse-cdn.com/v4/letter/t/41988e/32.png) [@Toshio\_Kuratomi](https://forum.ansible.com/u/Toshio_Kuratomi)\
**Post date:** [December 13, 2015, 1:58pm UTC](https://forum.ansible.com/t/replace-range-with-enumerate/19997/2 "2015-12-13T13:58:22Z")

</div>

> In the link  
> [https://github.com/ansible/ansible/blob/devel/lib/ansible/cli/\_\_init\_\_.py#L84](https://github.com/ansible/ansible/blob/devel/lib/ansible/cli/ __init__.py#L84),
> 
> &nbsp;&nbsp;&nbsp;&nbsp;for i in range(0, len(self.arg)):
> 
> is bit redundant, when you can just do range(len(self.arg))
> 
> But its better (and more Pythonic) to use enumerate:
> 
> for index, item in enumerate(self.args):  
> &nbsp;&nbsp;&nbsp;&nbsp;arg = item  
> &nbsp;&nbsp;&nbsp;&nbsp;if arg in self.VALID\_ACTIONS:  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;self.action = arg  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;del self.args[index]  
> &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;break

You are correct, enumerate would be more pythonic. However, do:

for index, arg in enumerate(self.args)  
&nbsp;&nbsp;&nbsp;&nbsp;if arg in self.VALID\_ACTIONS:  
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;[...]

That way you get rid of an assignment statement.

I did have to check whether deleting from the list as we were looping  
over enumerate would raise a RuntimeError like it will when you delete  
from a dict. It does not so we won't have any problems there.

Breaking out of the loop immediately after deleting protects this code  
from the index getting out of sync with the actual position that we  
are operating on in the list (A problem that would have affected the  
range() based code as well).

So if you want to pull request this, that is fine.

> Also, on another note, consider using for-else to replace:
> 
> if not self.action:  
> &nbsp;&nbsp;&nbsp;&nbsp;raise AnsibleOptionsError("Missing required action")
> 
> with this:
> 
> for:
> 
> &nbsp;&nbsp;&nbsp;&nbsp;...
> 
> else:
> 
> &nbsp;&nbsp;&nbsp;&nbsp;raise AnsibleOptionsError("Missing required action")

Since for-else is not present in other languages I like to be sparing  
in its use. If I do use it I always add a comment that explains what  
it is (otherwise the next programmer to look at the code is likely to  
think the code is supposed to go with an if statement within the loop  
and reindent it. So if you make this change in your PR, I'd suggest:

else: # This is a for-else: [http://bit.ly/1ElPkyg](http://bit.ly/1ElPkyg)  
&nbsp;&nbsp;&nbsp;&nbsp;raise AnsibleOptionsError("Missing required action")

-Toshio
