commit d50b8a06744a8476bc16c04268fef75b20712b2b Author: Damian Johnson <atagar@torproject.org> Date: Tue Dec 4 08:21:09 2012 -0800 Purging use of the variable 'i' A pox upon whoever taught me to use variables like i, j, and k for loop constants. It's ungrepable and a practice that should be purged via fire and pitchforks. I'm keeping it in for list interpolation (since it's localized to a single line), but replacing it with 'index' everywhere else. I'm also replacing a few points that do... for i in range(len(my_list)): my_item = my_list[i] ... with calls to enumerate(). --- stem/util/connection.py | 6 +++--- test/check_whitespace.py | 18 +++++++++--------- test/integ/control/base_controller.py | 2 +- test/mocking.py | 4 ++-- .../networkstatus/directory_authority.py | 4 ++-- test/unit/descriptor/networkstatus/document_v3.py | 20 +++++++++----------- test/unit/descriptor/router_status_entry.py | 4 ++-- test/unit/exit_policy/policy.py | 10 +++++----- test/unit/response/authchallenge.py | 4 ++-- test/unit/response/control_message.py | 14 +++++++------- test/unit/response/events.py | 2 +- test/unit/version.py | 8 ++++---- 12 files changed, 47 insertions(+), 49 deletions(-) diff --git a/stem/util/connection.py b/stem/util/connection.py index a5afc2b..a21cde8 100644 --- a/stem/util/connection.py +++ b/stem/util/connection.py @@ -142,9 +142,9 @@ def expand_ipv6_address(address): address = address.replace("::", "::" + ":" * missing_groups) # inserts missing zeros - for i in xrange(8): - start = i * 5 - end = address.index(":", start) if i != 7 else len(address) + for index in xrange(8): + start = index * 5 + end = address.index(":", start) if index != 7 else len(address) missing_zeros = 4 - (end - start) if missing_zeros > 0: diff --git a/test/check_whitespace.py b/test/check_whitespace.py index fa26a09..7f7b46e 100644 --- a/test/check_whitespace.py +++ b/test/check_whitespace.py @@ -44,8 +44,8 @@ def get_issues(base_path = DEFAULT_TARGET): has_with_import, given_with_warning = False, False is_block_comment = False - for i in xrange(len(lines)): - whitespace, content = re.match("^(\s*)(.*)$", lines[i]).groups() + for index, line in enumerate(lines): + whitespace, content = re.match("^(\s*)(.*)$", line).groups() if '"""' in content: is_block_comment = not is_block_comment @@ -54,23 +54,23 @@ def get_issues(base_path = DEFAULT_TARGET): has_with_import = True elif content.startswith("with ") and content.endswith(":") \ and not has_with_import and not given_with_warning and not is_block_comment: - file_issues.append((i + 1, "missing 'with' import (from __future__ import with_statement)")) + file_issues.append((index + 1, "missing 'with' import (from __future__ import with_statement)")) given_with_warning = True if "\t" in whitespace: - file_issues.append((i + 1, "indentation has a tab")) + file_issues.append((index + 1, "indentation has a tab")) elif "\r" in content: - file_issues.append((i + 1, "contains a windows newline")) + file_issues.append((index + 1, "contains a windows newline")) elif content != content.rstrip(): - file_issues.append((i + 1, "line has trailing whitespace")) + file_issues.append((index + 1, "line has trailing whitespace")) elif content == '': # empty line, check its indentation against the previous and next line # with content next_indent = 0 - for k in xrange(i + 1, len(lines)): - future_whitespace, future_content = re.match("^(\s*)(.*)$", lines[k]).groups() + for future_index in xrange(index + 1, len(lines)): + future_whitespace, future_content = re.match("^(\s*)(.*)$", lines[future_index]).groups() if future_content: next_indent = len(future_whitespace) @@ -86,7 +86,7 @@ def get_issues(base_path = DEFAULT_TARGET): else: msg = msg % ("%i or %i" % (next_indent, prev_indent)) - file_issues.append((i + 1, msg)) + file_issues.append((index + 1, msg)) else: # we had content and it's fine, making a note of its indentation prev_indent = len(whitespace) diff --git a/test/integ/control/base_controller.py b/test/integ/control/base_controller.py index eebb788..900afad 100644 --- a/test/integ/control/base_controller.py +++ b/test/integ/control/base_controller.py @@ -119,7 +119,7 @@ class TestBaseController(unittest.TestCase): msg_thread.setDaemon(True) msg_thread.start() - for i in xrange(100): + for index in xrange(100): controller.connect() controller.close() diff --git a/test/mocking.py b/test/mocking.py index e48ce64..6f05411 100644 --- a/test/mocking.py +++ b/test/mocking.py @@ -380,8 +380,8 @@ def get_all_combinations(attr, include_empty = False): if include_empty: yield () seen = set() - for i in xrange(1, len(attr) + 1): - product_arg = [attr for _ in xrange(i)] + for index in xrange(1, len(attr) + 1): + product_arg = [attr for _ in xrange(index)] for item in itertools.product(*product_arg): # deduplicate, sort, and only provide if we haven't seen it yet diff --git a/test/unit/descriptor/networkstatus/directory_authority.py b/test/unit/descriptor/networkstatus/directory_authority.py index f01604f..545d72f 100644 --- a/test/unit/descriptor/networkstatus/directory_authority.py +++ b/test/unit/descriptor/networkstatus/directory_authority.py @@ -99,8 +99,8 @@ class TestDirectoryAuthority(unittest.TestCase): lines = get_directory_authority(content = True).split("\n") - for i in xrange(len(lines)): - content = "\n".join(lines[:i] + [lines[i]] + lines[i:]) + for index, duplicate_line in enumerate(lines): + content = "\n".join(lines[:index] + [duplicate_line] + lines[index:]) self.assertRaises(ValueError, DirectoryAuthority, content) authority = DirectoryAuthority(content, False) diff --git a/test/unit/descriptor/networkstatus/document_v3.py b/test/unit/descriptor/networkstatus/document_v3.py index 0396754..9eb45a5 100644 --- a/test/unit/descriptor/networkstatus/document_v3.py +++ b/test/unit/descriptor/networkstatus/document_v3.py @@ -161,14 +161,14 @@ class TestNetworkStatusDocument(unittest.TestCase): attr = {"vote-status": "consensus"} if is_consensus else {"vote-status": "vote"} lines = get_network_status_document_v3(attr, content = True).split("\n") - for i in xrange(len(lines) - 1): + for index in xrange(len(lines) - 1): # once we reach the crypto blob we're done since swapping those won't # be detected - if lines[i].startswith(CRYPTO_BLOB[1:10]): break + if lines[index].startswith(CRYPTO_BLOB[1:10]): break # swaps this line with the one after it test_lines = list(lines) - test_lines[i], test_lines[i + 1] = test_lines[i + 1], test_lines[i] + test_lines[index], test_lines[index + 1] = test_lines[index + 1], test_lines[index] content = "\n".join(test_lines) self.assertRaises(ValueError, NetworkStatusDocumentV3, content) @@ -184,18 +184,18 @@ class TestNetworkStatusDocument(unittest.TestCase): attr = {"vote-status": "consensus"} if is_consensus else {"vote-status": "vote"} lines = get_network_status_document_v3(attr, content = True).split("\n") - for i in xrange(len(lines)): + for index, line in enumerate(lines): # Stop when we hit the 'directory-signature' for a couple reasons... # - that is the one field that can validly appear multiple times # - after it is a crypto blob, which won't trigger this kind of # validation failure test_lines = list(lines) - if test_lines[i].startswith("directory-signature "): + if line.startswith("directory-signature "): break # duplicates the line - test_lines.insert(i, test_lines[i]) + test_lines.insert(index, line) content = "\n".join(test_lines) self.assertRaises(ValueError, NetworkStatusDocumentV3, content) @@ -548,11 +548,9 @@ class TestNetworkStatusDocument(unittest.TestCase): weight_entries, expected = [], {} - for i in xrange(len(BANDWIDTH_WEIGHT_ENTRIES)): - key, value = BANDWIDTH_WEIGHT_ENTRIES[i], i - 5 - - weight_entries.append("%s=%i" % (key, value)) - expected[key] = value + for index, key in enumerate(BANDWIDTH_WEIGHT_ENTRIES): + weight_entries.append("%s=%i" % (key, index - 5)) + expected[key] = index - 5 document = get_network_status_document_v3({"bandwidth-weights": " ".join(weight_entries)}) self.assertEquals(expected, document.bandwidth_weights) diff --git a/test/unit/descriptor/router_status_entry.py b/test/unit/descriptor/router_status_entry.py index d971906..27eb76d 100644 --- a/test/unit/descriptor/router_status_entry.py +++ b/test/unit/descriptor/router_status_entry.py @@ -148,8 +148,8 @@ class TestRouterStatusEntry(unittest.TestCase): lines = get_router_status_entry_v3(content = True).split("\n") - for i in xrange(len(lines)): - content = "\n".join(lines[:i] + [lines[i]] + lines[i:]) + for index, duplicate_line in enumerate(lines): + content = "\n".join(lines[:index] + [duplicate_line] + lines[index:]) self.assertRaises(ValueError, RouterStatusEntryV3, content) entry = RouterStatusEntryV3(content, False) diff --git a/test/unit/exit_policy/policy.py b/test/unit/exit_policy/policy.py index 6f4b5a9..0714ef9 100644 --- a/test/unit/exit_policy/policy.py +++ b/test/unit/exit_policy/policy.py @@ -63,12 +63,12 @@ class TestExitPolicy(unittest.TestCase): policy = ExitPolicy('accept *:80', 'accept *:443', 'reject *:*') - for i in xrange(1, 500): - ip_addr = "%i.%i.%i.%i" % (i / 2, i / 2, i / 2, i / 2) - expected_result = i in (80, 443) + for index in xrange(1, 500): + ip_addr = "%i.%i.%i.%i" % (index / 2, index / 2, index / 2, index / 2) + expected_result = index in (80, 443) - self.assertEquals(expected_result, policy.can_exit_to(ip_addr, i)) - self.assertEquals(expected_result, policy.can_exit_to(port = i)) + self.assertEquals(expected_result, policy.can_exit_to(ip_addr, index)) + self.assertEquals(expected_result, policy.can_exit_to(port = index)) def test_is_exiting_allowed(self): test_inputs = { diff --git a/test/unit/response/authchallenge.py b/test/unit/response/authchallenge.py index 5560b03..19ce931 100644 --- a/test/unit/response/authchallenge.py +++ b/test/unit/response/authchallenge.py @@ -43,12 +43,12 @@ class TestAuthChallengeResponse(unittest.TestCase): auth_challenge_comp = VALID_RESPONSE.split() - for i in xrange(1, len(auth_challenge_comp)): + for index in xrange(1, len(auth_challenge_comp)): # Attempts to parse a message without this item. The first item is # skipped because, without the 250 code, the message won't be # constructed. - remaining_comp = auth_challenge_comp[:i] + auth_challenge_comp[i + 1:] + remaining_comp = auth_challenge_comp[:index] + auth_challenge_comp[index + 1:] control_message = mocking.get_message(' '.join(remaining_comp)) self.assertRaises(stem.ProtocolError, stem.response.convert, "AUTHCHALLENGE", control_message) diff --git a/test/unit/response/control_message.py b/test/unit/response/control_message.py index 68a5c42..115aed5 100644 --- a/test/unit/response/control_message.py +++ b/test/unit/response/control_message.py @@ -107,14 +107,14 @@ class TestControlMessage(unittest.TestCase): infonames_lines = [line + "\n" for line in GETINFO_INFONAMES.splitlines()] - for i in range(len(infonames_lines)): + for index, line in enumerate(infonames_lines): # replace the CRLF for the line - infonames_lines[i] = infonames_lines[i].rstrip("\r\n") + "\n" + infonames_lines[index] = line.rstrip("\r\n") + "\n" test_socket_file = StringIO.StringIO("".join(infonames_lines)) self.assertRaises(stem.ProtocolError, stem.socket.recv_message, test_socket_file) # puts the CRLF back - infonames_lines[i] = infonames_lines[i].rstrip("\n") + "\r\n" + infonames_lines[index] = infonames_lines[index].rstrip("\n") + "\r\n" # sanity check the above test isn't broken due to leaving infonames_lines # with invalid data @@ -126,12 +126,12 @@ class TestControlMessage(unittest.TestCase): Checks parsing for responses where the header is missing a digit or divider. """ - for i in range(len(EVENT_BW)): + for index in range(len(EVENT_BW)): # makes test input with that character missing or replaced - removal_test_input = EVENT_BW[:i] + EVENT_BW[i + 1:] - replacement_test_input = EVENT_BW[:i] + "#" + EVENT_BW[i + 1:] + removal_test_input = EVENT_BW[:index] + EVENT_BW[index + 1:] + replacement_test_input = EVENT_BW[:index] + "#" + EVENT_BW[index + 1:] - if i < 4 or i >= (len(EVENT_BW) - 2): + if index < 4 or index >= (len(EVENT_BW) - 2): # dropping the character should cause an error if... # - this is part of the message prefix # - this is disrupting the line ending diff --git a/test/unit/response/events.py b/test/unit/response/events.py index 6fbd907..2ce5167 100644 --- a/test/unit/response/events.py +++ b/test/unit/response/events.py @@ -213,7 +213,7 @@ class TestEvents(unittest.TestCase): self.assertEqual("sent: 25, received: 15", msg) def event_sender(): - for i in xrange(3): + for index in xrange(3): print_bw(self._get_event("650 BW 15 25")) time.sleep(0.05) diff --git a/test/unit/version.py b/test/unit/version.py index 0ff39dd..df4caa5 100644 --- a/test/unit/version.py +++ b/test/unit/version.py @@ -194,8 +194,8 @@ class TestVersion(unittest.TestCase): requirements = stem.version.VersionRequirements() requirements.in_range(Version("0.2.2.0"), Version("0.2.3.0")) - for i in xrange(0, 100): - self.assertTrue(Version("0.2.2.%i" % i).meets_requirements(requirements)) + for index in xrange(0, 100): + self.assertTrue(Version("0.2.2.%i" % index).meets_requirements(requirements)) def test_requirements_multiple_rules(self): """ @@ -210,8 +210,8 @@ class TestVersion(unittest.TestCase): self.assertTrue(Version("0.2.3.0").meets_requirements(requirements)) self.assertFalse(Version("0.2.2.0").meets_requirements(requirements)) - for i in xrange(0, 100): - self.assertFalse(Version("0.2.2.%i" % i).meets_requirements(requirements)) + for index in xrange(0, 100): + self.assertFalse(Version("0.2.2.%i" % index).meets_requirements(requirements)) def assert_versions_match(self, version, major, minor, micro, patch, status, extra): """