kennethreitz/dive-into-python3
1
0
Fork 0
mirror of https://github.com/kennethreitz/dive-into-python3.git synced 2026-06-05 15:00:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

you wouldn't believe me if I told you

Browse Source
This commit is contained in:
Mark Pilgrim
2009-06-05 23:39:50 -04:00
parent cbdb346531
commit 654b102d74
64 changed files with 724 additions and 764 deletions
Download Patch File Download Diff File Expand all files Collapse all files
refactoring.html
+35 -34
View File
@@ -23,7 +23,7 @@ body{counter-reset:h1 10}
<p class=f>Despite your best efforts to write comprehensive unit tests, bugs happen. What do I mean by &#8220;bug&#8221;? A bug is a test case you haven&#8217;t written yet.
<pre class=screen><samp class=p>>>> </samp><kbd>import roman7</kbd>
<a><samp class=p>>>> </samp><kbd>roman7.from_roman("")</kbd> <span>&#x2460;</span></a>
<a><samp class=p>>>> </samp><kbd>roman7.from_roman('')</kbd> <span>&#x2460;</span></a>
<samp>0</samp></pre>
<ol>
<li>Remember in the [FIXME-xref] previous section when you kept seeing that an empty string would match the regular expression you were using to check for valid Roman numerals? Well, it turns out that this is still true for the final version of the regular expression. And that&#8217;s a bug; you want an empty string to raise an <code>InvalidRomanNumeralError</code> exception just like any other sequence of characters that don&#8217;t represent a valid Roman numeral.
@@ -36,8 +36,8 @@ body{counter-reset:h1 10}
.
.
def testBlank(self):
"""from_roman should fail with blank string"""
<a> self.assertRaises(roman6.InvalidRomanNumeralError, roman6.from_roman, "") <span>&#x2460;</span></a></code></pre>
'''from_roman should fail with blank string'''
<a> self.assertRaises(roman6.InvalidRomanNumeralError, roman6.from_roman, '') <span>&#x2460;</span></a></code></pre>
<ol>
<li>Pretty simple stuff here. Call <code>from_roman()</code> with an empty string and make sure it raises an <code>InvalidRomanNumeralError</code> exception. The hard part was finding the bug; now that you know about it, testing for it is the easy part.
</ol>
@@ -62,7 +62,7 @@ FAIL: from_roman should fail with blank string
----------------------------------------------------------------------
Traceback (most recent call last):
File "romantest8.py", line 117, in test_blank
self.assertRaises(roman8.InvalidRomanNumeralError, roman8.from_roman, "")
self.assertRaises(roman8.InvalidRomanNumeralError, roman8.from_roman, '')
<mark>AssertionError: InvalidRomanNumeralError not raised by from_roman</mark>
----------------------------------------------------------------------
@@ -73,7 +73,7 @@ FAILED (failures=1)</samp></pre>
<p><em>Now</em> you can fix the bug.
<pre><code>def from_roman(s):
"""convert Roman numeral to integer"""
'''convert Roman numeral to integer'''
<a> if not s: <span>&#x2460;</span></a>
raise InvalidRomanNumeralError, 'Input can not be blank'
if not re.search(romanNumeralPattern, s):
@@ -137,7 +137,7 @@ class KnownValues(unittest.TestCase):
class ToRomanBadInput(unittest.TestCase):
def test_too_large(self):
"""to_roman should fail with large input"""
'''to_roman should fail with large input'''
<a> self.assertRaises(roman8.OutOfRangeError, roman8.to_roman, 5000) <span>&#x2461;</span></a>
.
@@ -146,7 +146,7 @@ class ToRomanBadInput(unittest.TestCase):
class FromRomanBadInput(unittest.TestCase):
def test_too_many_repeated_numerals(self):
"""from_roman should fail with too many repeated numerals"""
'''from_roman should fail with too many repeated numerals'''
<a> for s in ('MMMMM', 'DD', 'CCCC', 'LL', 'XXXX', 'VV', 'IIII'): <span>&#x2462;</span></a>
self.assertRaises(roman8.InvalidRomanNumeralError, roman8.from_roman, s)
@@ -156,7 +156,7 @@ class FromRomanBadInput(unittest.TestCase):
class RoundtripCheck(unittest.TestCase):
def test_roundtrip(self):
"""from_roman(to_roman(n))==n for all n"""
'''from_roman(to_roman(n))==n for all n'''
<a> for integer in range(1, 5000): <span>&#x2463;</span></a>
numeral = roman8.to_roman(integer)
result = roman8.from_roman(numeral)
@@ -192,7 +192,7 @@ Traceback (most recent call last):
File "romantest9.py", line 82, in test_from_roman_known_values
result = roman9.from_roman(numeral)
File "C:\home\diveintopython3\examples\roman9.py", line 60, in from_roman
raise InvalidRomanNumeralError("Invalid Roman numeral: {0}".format(s))
raise InvalidRomanNumeralError('Invalid Roman numeral: {0}'.format(s))
<mark>roman9.InvalidRomanNumeralError: Invalid Roman numeral: MMMM</mark>
======================================================================
@@ -202,7 +202,7 @@ Traceback (most recent call last):
File "romantest9.py", line 76, in test_to_roman_known_values
result = roman9.to_roman(integer)
File "C:\home\diveintopython3\examples\roman9.py", line 42, in to_roman
raise OutOfRangeError("number out of range (must be 0..3999)")
raise OutOfRangeError('number out of range (must be 0..3999)')
<mark>roman9.OutOfRangeError: number out of range (must be 0..3999)</mark>
======================================================================
@@ -212,7 +212,7 @@ Traceback (most recent call last):
File "romantest9.py", line 131, in testSanity
numeral = roman9.to_roman(integer)
File "C:\home\diveintopython3\examples\roman9.py", line 42, in to_roman
raise OutOfRangeError("number out of range (must be 0..3999)")
raise OutOfRangeError('number out of range (must be 0..3999)')
<mark>roman9.OutOfRangeError: number out of range (must be 0..3999)</mark>
----------------------------------------------------------------------
@@ -229,7 +229,7 @@ FAILED (errors=3)</samp></pre>
<p class=d>[<a href=examples/roman9.py>download <code>roman9.py</code></a>]
<pre><code>
roman_numeral_pattern = re.compile("""
roman_numeral_pattern = re.compile('''
^ # beginning of string
<a> M{0,4} # thousands - 0 to 4 M's <span>&#x2460;</span></a>
(CM|CD|D?C{0,3}) # hundreds - 900 (CM), 400 (CD), 0-300 (0 to 3 C's),
@@ -239,16 +239,16 @@ roman_numeral_pattern = re.compile("""
(IX|IV|V?I{0,3}) # ones - 9 (IX), 4 (IV), 0-3 (0 to 3 I's),
# or 5-8 (V, followed by 0 to 3 I's)
$ # end of string
""", re.VERBOSE)
''', re.VERBOSE)
def to_roman(n):
"""convert integer to Roman numeral"""
'''convert integer to Roman numeral'''
<a> if not (0 < n < 5000): <span>&#x2461;</span></a>
raise OutOfRangeError("number out of range (must be 0..4999)")
raise OutOfRangeError('number out of range (must be 0..4999)')
if not isinstance(n, int):
raise NotIntegerError("non-integers can not be converted")
raise NotIntegerError('non-integers can not be converted')
result = ""
result = ''
for numeral, integer in roman_numeral_map:
while n >= integer:
result += numeral
@@ -299,7 +299,7 @@ Ran 12 tests in 0.203s
<p>Refactoring is the process of taking working code and making it work better. Usually, &#8220;better&#8221; means &#8220;faster&#8221;, although it can also mean &#8220;using less memory&#8221;, or &#8220;using less disk space&#8221;, or simply &#8220;more elegantly&#8221;. Whatever it means to you, to your project, in your environment, refactoring is important to the long-term health of any program.
<p>Here, &#8220;better&#8221; means both &#8220;faster&#8221; and &#8220;easier to maintain.&#8221; Specifically, the <code>from_roman()</code> function is slower and more complex than I&#8217;d like, because of that big nasty regular expression that you use to validate Roman numerals. Now, you might think, "Sure, the regular expression is big and hairy, but how else am I supposed to validate that an arbitrary string is a valid a Roman numeral?"
<p>Here, &#8220;better&#8221; means both &#8220;faster&#8221; and &#8220;easier to maintain.&#8221; Specifically, the <code>from_roman()</code> function is slower and more complex than I&#8217;d like, because of that big nasty regular expression that you use to validate Roman numerals. Now, you might think, &#8220;Sure, the regular expression is big and hairy, but how else am I supposed to validate that an arbitrary string is a valid a Roman numeral?&#8221;
<p>Answer: there&#8217;s only 5000 of them; why don&#8217;t you just build a lookup table? This idea gets even better when you realize that <em>you don&#8217;t need to use regular expressions at all</em>. As you build the lookup table for converting integers to Roman numerals, you can build the reverse lookup table to convert Roman numerals to integers. By the time you need to check whether an arbitrary string is a valid Roman numeral, you will have collected all the valid Roman numerals. &#8220;Validating&#8221; is reduced to a single dictionary lookup.
@@ -328,26 +328,26 @@ to_roman_table = [ None ]
from_roman_table = {}
def to_roman(n):
"""convert integer to Roman numeral"""
'''convert integer to Roman numeral'''
if not (0 < n < 5000):
raise OutOfRangeError("number out of range (must be 1..4999)")
raise OutOfRangeError('number out of range (must be 1..4999)')
if int(n) != n:
raise NotIntegerError("non-integers can not be converted")
raise NotIntegerError('non-integers can not be converted')
return to_roman_table[n]
def from_roman(s):
"""convert Roman numeral to integer"""
'''convert Roman numeral to integer'''
if not isinstance(s, str):
raise InvalidRomanNumeralError("Input must be a string")
raise InvalidRomanNumeralError('Input must be a string')
if not s:
raise InvalidRomanNumeralError("Input can not be blank")
raise InvalidRomanNumeralError('Input can not be blank')
if s not in from_roman_table:
raise InvalidRomanNumeralError("Invalid Roman numeral: {0}".format(s))
raise InvalidRomanNumeralError('Invalid Roman numeral: {0}'.format(s))
return from_roman_table[s]
def build_lookup_tables():
def to_roman(n):
result = ""
result = ''
for numeral, integer in roman_numeral_map:
if n >= integer:
result = numeral
@@ -379,7 +379,7 @@ from_roman_table = {}
.
def build_lookup_tables():
<a> def to_roman(n): <span>&#x2460;</span></a>
result = ""
result = ''
for numeral, integer in roman_numeral_map:
if n >= integer:
result = numeral
@@ -402,21 +402,21 @@ def build_lookup_tables():
<p>Once the lookup tables are built, the rest of the code is both easy and fast.
<pre><code>def to_roman(n):
"""convert integer to Roman numeral"""
'''convert integer to Roman numeral'''
if not (0 < n < 5000):
raise OutOfRangeError("number out of range (must be 1..4999)")
raise OutOfRangeError('number out of range (must be 1..4999)')
if int(n) != n:
raise NotIntegerError("non-integers can not be converted")
raise NotIntegerError('non-integers can not be converted')
<a> return to_roman_table[n] <span>&#x2460;</span></a>
def from_roman(s):
"""convert Roman numeral to integer"""
'''convert Roman numeral to integer'''
if not isinstance(s, str):
raise InvalidRomanNumeralError("Input must be a string")
raise InvalidRomanNumeralError('Input must be a string')
if not s:
raise InvalidRomanNumeralError("Input can not be blank")
raise InvalidRomanNumeralError('Input can not be blank')
if s not in from_roman_table:
raise InvalidRomanNumeralError("Invalid Roman numeral: {0}".format(s))
raise InvalidRomanNumeralError('Invalid Roman numeral: {0}'.format(s))
<a> return from_roman_table[s] <span>&#x2461;</span></a></code></pre>
<ol>
<li>After doing the same bounds checking as before, the <code>to_roman()</code> function simply finds the appropriate value in the lookup table and returns it.
@@ -473,6 +473,7 @@ OK</samp></pre>
<li>Refactoring mercilessly to improve performance, scalability, readability, maintainability, or whatever other -ility you&#8217;re lacking
</ul>
<p class=v><a rel=prev class=todo><span>&#x261C;</span></a> <a rel=next class=todo><span>&#x261E;</span></a>
<p class=c>&copy; 2001&ndash;9 <a href=about.html>Mark Pilgrim</a>
<script src=j/jquery.js></script>
<script src=j/dip3.js></script>