Re: [Tutor] don't repeat yourself; question about code optimization CORRECTION
On 7/23/07, Bob Gailer [EMAIL PROTECTED] wrote: A correction to the code at the end. The test of self.total_num_of_items should precede the pop(0) Bob, I spent today studying what you've been telling me and I put the finishing touches to make your code pass my battery of tests. It still is repetitive, i.e., table.append(tuple(row)), but I've found guidance in what Alan had to say about code readability and easy maintenance, and am no longer going to press the matter: class Table_Creator(object): def __init__(self, total_num_of_items, max_num_of_items_per_row): self.given_items = range(total_num_of_items) self.max_num_of_items_per_row = max_num_of_items_per_row def create_grid(self): table = [] while True: row = [] while len(row) self.max_num_of_items_per_row: if not self.given_items: if row: table.append(tuple(row)) return table row.append(self.given_items.pop(0)) table.append(tuple(row)) ___ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor
Re: [Tutor] don't repeat yourself; question about code optimization
On 7/23/07, Alan Gauld [EMAIL PROTECTED] wrote: The prime directive of coding is make it readable! The DRY principle is just that a principle. If repeating makes for more maintainable or readable code then repeat yourself. Remember 80% of the cost of software is in maintenance not initial development. DRY is one way to improve maintainablility PS The most serious problem with your code from my perpspective is that your variable names are way too long. That affects maintenance and readability more that the repetition (and in the case of email it causes line wrapping that makes it even worse!) Finally the class is not a good class in an OOP sense since it is nearly a verb. It is better done as a simple function in my view, just pass in the parameters to the create method. Like this: def create_table(num_items, row_size): start = 0 table = [] num_rows = num_items/row_size for n in range(num_rows): row = [num for num in range(start,start+row_size)] table.append(tuple(row)) start += row_size return table Or better still build a table class that knows how to create itself, but also knows how to maniplulate itself too - doing whatever it is you intend doing to the table you just created! This reflects another programming rule - the law of demeter - one of the fundamentals of OOP. Alan, I spent today going over what you took the time to patiently illustrate to me, and I believe I learned the following things: - code readability and ease of maintenance are the primary goals of a computer programmer - classes that are very nearly verbs should be converted to functions, unless you can write a class that knows how to create and manipulate itself, following the law of demeter About the last bit of code, where you propose to create the table using list comprehension, I did want to capture partial rows, so I put the finishing touches to your work so it could. When I subjected the code to a battery of tests, it failed in the following way: def create_table(num_items, row_size): start = 0 table = [] num_rows = (num_items/row_size) + (num_items%row_size) for n in range(num_rows): row = [num for num in range(num_items)[start:start+row_size]] table.append(tuple(row)) start += row_size return table def test_harness(): assert create_table(3, 1) == [(0,), (1,), (2,)] assert create_table(4, 1) == [(0,), (1,), (2,), (3,)] assert create_table(1, 2) == [(0,)] assert create_table(2, 2) == [(0, 1)] assert create_table(4, 2) == [(0, 1), (2, 3)] assert create_table(5, 2) == [(0, 1), (2, 3), (4, )] assert create_table(5, 3) == [(0, 1, 2), (3, 4)] assert create_table(7, 4) == [(0, 1, 2, 3), (4, 5, 6)] assert create_table(5, 3) == [(0, 1, 2), (3, 4)] AssertionError I know that my method of calculating the number of rows is faulty, but I'm not sure how to correct it. ___ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor
Re: [Tutor] don't repeat yourself; question about code optimization CORRECTION
[EMAIL PROTECTED] wrote: On 7/23/07, *Bob Gailer* [EMAIL PROTECTED] mailto:[EMAIL PROTECTED] wrote: A correction to the code at the end. The test of self.total_num_of_items should precede the pop(0) Bob, I spent today studying what you've been telling me and I put the finishing touches to make your code pass my battery of tests. It still is repetitive, i.e., table.append(tuple(row)), I see you're also repeating the use of = quite a bit, as well as multiple uses of the word if. Maybe you could work on that. Sorry for the sarcasm, but there's a big different between repeating an assignment and a type coersion versus a multi line block of code that could be converted to a function. :-) but I've found guidance in what Alan had to say about code readability and easy maintenance, and am no longer going to press the matter: class Table_Creator(object): def __init__(self, total_num_of_items, max_num_of_items_per_row): self.given_items = range(total_num_of_items) self.max_num_of_items_per_row = max_num_of_items_per_row def create_grid(self): table = [] while True: row = [] while len(row) self.max_num_of_items_per_row: if not self.given_items: if row: table.append(tuple(row)) return table row.append(self.given_items.pop(0)) table.append(tuple(row)) ___ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor ___ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor
Re: [Tutor] don't repeat yourself; question about code optimization
assert create_table(5, 3) == [(0, 1, 2), (3, 4)] AssertionError I know that my method of calculating the number of rows is faulty, but I'm not sure how to correct it. num_rows = (num_items/row_size) + (num_items%row_size) This uses integer division then adds the remainder, so for your example of 5,3 we get 5/3 = 1 5%3 = 2 So rows = 3 - wrong! All you want to do is add one if the modulus is not zero, so num_rows = (num_items/row_size) if num_items % row_size 0: num_rows += 1 Should do what you want. HTH, Alan G. ___ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor
Re: [Tutor] don't repeat yourself; question about code optimization CORRECTION
[EMAIL PROTECTED] wrote: On 7/23/07, *Bob Gailer* [EMAIL PROTECTED] mailto:[EMAIL PROTECTED] wrote: A correction to the code at the end. The test of self.total_num_of_items should precede the pop(0) Bob, I spent today studying what you've been telling me and I put the finishing touches to make your code pass my battery of tests. It still is repetitive, i.e., table.append(tuple(row)), but I've found guidance in what Alan had to say about code readability and easy maintenance, and am no longer going to press the matter: class Table_Creator(object): def __init__(self, total_num_of_items, max_num_of_items_per_row): self.given_items = range(total_num_of_items) self.max_num_of_items_per_row = max_num_of_items_per_row def create_grid(self): table = [] while True: row = [] while len(row) self.max_num_of_items_per_row: if not self.given_items: if row: table.append(tuple(row)) return table row.append(self.given_items.pop(0)) table.append(tuple(row)) A bit more juggling leads to more simplification: def create_grid(self): table = [] while self.given_items: row = [] while len(row) self.max_num_of_items_per_row and self.given_items: row.append(self.given_items.pop(0)) table.append(tuple(row)) return table Testing self.given_items twice is unavoidable. -- Bob Gailer 510-978-4454 Oakland, CA 919-636-4239 Chapel Hill, NC ___ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor
Re: [Tutor] don't repeat yourself; question about code optimization
On 7/20/07, Bob Gailer [EMAIL PROTECTED] wrote: Take advantage of slicing: def create_grid(self): table = [] for i in range(0, len(self.total_num_of_items), self.max_num_of_items_per_row): table.append(tuple(self.total_num_of_items[i : i + self.max_num_of_items_per_row])) return table simply amazing. Thank you. OK - to address your original question: def create_grid(self): table = [] while self.total_num_of_items: row = [] count = 0 while count self.max_num_of_items_per_row and self.total_num_of_items: row.append(self.total_num_of_items.pop(0)) count += 1 table.append(tuple(row)) return table At first I regarded you with quiet awe, but then the nitpick in me saw the two while self.total_num_of_item statements, and I was less pleased. However, I see this as a doable challenge you have given me, and I will attempt to optimize your revisions. Thanks again. ___ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor
Re: [Tutor] don't repeat yourself; question about code optimization
[EMAIL PROTECTED] wrote one part where I believe I violate the prime directive of coding, which is not to repeat yourself: The prime directive of coding is make it readable! The DRY principle is just that a principle. If repeating makes for more maintainable or readable code then repeat yourself. Getting too hung up on a catchy acronym is a dangerous thing IMHO. It can lead to too much effort going into satisfying the principle and not enough into thinking about the problem at hand and how the code will be maintained. Remember 80% of the cost of software is in maintenance not initial development. DRY is one way to improve maintainablility and that's its purpose - to avoid having to fix things in two places - but it is not a rule that must be slavishly followed at the expense of readability/maintainability. It's in the same category as Do not use GOTO, Do not use global variables, Functions should be less than 25 lines long These are all useful principles which usually improve code quality but there are exceptions in every case. Now in your example removing repetition will improve things slightly, but I am always concerned when I see terms like prime directive of coding being applied to something like DRY. PS The most serious problem with your code from my perpspective is that your variable names are way too long. That affects maintenance and readability more that the repetition (and in the case of email it causes line wrapping that makes it even worse!) The names are also not accurate of their function, for example total_num_of_items is not a number at all but a list of numbers... I'd therefore suggest a rewrite of your init method like: def __init__(self, num_of_items, max_per_row): self.numbers = range(num_of_items) self.max_per_row = max_per_row And the modified create method looks like: def create_grid(self): table = [] row = [] count = 0 while self.numbers: row.append(self.numbers.pop(0)) count += 1 if (not self.numbers) or (count == self.max_per_row): table.append(tuple(row)) row = [] count = 0 return table Which makes the indentation error more obvious... The use of a while loop here could be replaced by a Python for loop which eliminates your repetition and is more natural: def create_grid(self): table = [] row = [] for number in self.numbers: row.append(number) if len(row) == self.max_per_row: table.append(tuple(row)) row = [] if len(row) != 0 # not sure if you want partially completed rows or not table.append(tuple(row)) return table However, since you are in effect trying to create a list of lists I suggest a combination of list comprehension and slicing would be a better solution. def create_grid(self): start = 0 table = [] num_rows = len(self.numbers)/max_per_row# this should really be in init! for n in range(num_rows): row = [num for num in self.numbers[start:start+self.max_per_row]] table.append(row) start += self.max_per_row return table Which happens to meet the DRY principle. But we got there, not by trying to avoid DRY but by improving the algorithm and structure of the code. DRY was merely a side efffect. Finally the class is not a good class in an OOP sense since it is nearly a verb. It is better done as a simple function in my view, just pass in the parameters to the create method. Like this: def create_table(num_items, row_size): start = 0 table = [] num_rows = num_items/row_size for n in range(num_rows): row = [num for num in range(start,start+row_size)] table.append(tuple(row)) start += row_size return table Or better still build a table class that knows how to create itself, but also knows how to maniplulate itself too - doing whatever it is you intend doing to the table you just created! This reflects another programming rule - the law of demeter - one of the fundamentals of OOP. -- Alan Gauld Author of the Learn to Program web site http://www.freenetpages.co.uk/hp/alan.gauld ___ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor
Re: [Tutor] don't repeat yourself; question about code optimization
[EMAIL PROTECTED] wrote: On 7/20/07, *Bob Gailer* [EMAIL PROTECTED] mailto:[EMAIL PROTECTED] wrote: Take advantage of slicing: def create_grid(self): table = [] for i in range(0, len( self.total_num_of_items), self.max_num_of_items_per_row): table.append(tuple(self.total_num_of_items[i : i + self.max_num_of_items_per_row ])) return table simply amazing. Thank you. One of the many shifts one makes in adjusting to new language features. I made such a shift in 1974 when I switched from FORTRAN and PL/I to APL. OK - to address your original question: def create_grid(self): table = [] while self.total_num_of_items: row = [] count = 0 while count self.max_num_of_items_per_row and self.total_num_of_items: row.append(self.total_num_of_items.pop(0)) count += 1 table.append(tuple(row)) return table At first I regarded you with quiet awe We gurus bask in attention. , but then the nitpick in me saw the two while self.total_num_of_item statements, and I was less pleased. Oh all right, that costs us one more statement. Independently we can get rid of count: def create_grid(self): table = [] while True: row = [] while len(row) self.max_num_of_items_per_row: row.append(self.total_num_of_items.pop(0)) if not self.total_num_of_items: return table table.append(tuple(row)) -- Bob Gailer 510-978-4454 Oakland, CA 919-636-4239 Chapel Hill, NC ___ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor
Re: [Tutor] don't repeat yourself; question about code optimization CORRECTION
A correction to the code at the end. The test of self.total_num_of_items should precede the pop(0) [EMAIL PROTECTED] wrote: On 7/20/07, *Bob Gailer* [EMAIL PROTECTED] mailto:[EMAIL PROTECTED] wrote: Take advantage of slicing: def create_grid(self): table = [] for i in range(0, len( self.total_num_of_items), self.max_num_of_items_per_row): table.append(tuple(self.total_num_of_items[i : i + self.max_num_of_items_per_row ])) return table simply amazing. Thank you. One of the many shifts one makes in adjusting to new language features. I made such a shift in 1974 when I switched from FORTRAN and PL/I to APL. OK - to address your original question: def create_grid(self): table = [] while self.total_num_of_items: row = [] count = 0 while count self.max_num_of_items_per_row and self.total_num_of_items: row.append(self.total_num_of_items.pop(0)) count += 1 table.append(tuple(row)) return table At first I regarded you with quiet awe We gurus bask in attention. , but then the nitpick in me saw the two while self.total_num_of_item statements, and I was less pleased. Oh all right, that costs us one more statement. Independently we can get rid of count: def create_grid(self): table = [] while True: row = [] while len(row) self.max_num_of_items_per_row: if not self.total_num_of_items: return table row.append(self.total_num_of_items.pop(0)) table.append(tuple(row)) -- Bob Gailer 510-978-4454 Oakland, CA 919-636-4239 Chapel Hill, NC ___ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor
[Tutor] don't repeat yourself; question about code optimization
dear fellow Python enthusiasts: in the last year I have been experimenting with Python, and I set out to create a function that, given a number of items and a maximum number of items per row, would generate a table of rows of items. However, there is one part where I believe I violate the prime directive of coding, which is not to repeat yourself: class Table_Creator(object): def __init__(self, given_num_of_items, max_num_of_items_per_row): self.total_num_of_items = range(given_num_of_items) self.max_num_of_items_per_row = max_num_of_items_per_row def create_grid(self): table = [] row = [] count = 0 while self.total_num_of_items: row.append(self.total_num_of_items.pop(0)) count += 1 if (not self.total_num_of_items) or (count == self.max_num_of_items_per_row): table.append(tuple(row)) row = [] count = 0 return table as you can see, I repeat the expressions row = [] and count = 0, and I would like to know if there is something I can do to avoid repetition in this case. ___ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor
Re: [Tutor] don't repeat yourself; question about code optimization
[EMAIL PROTECTED] wrote: dear fellow Python enthusiasts: in the last year I have been experimenting with Python, and I set out to create a function that, given a number of items and a maximum number of items per row, would generate a table of rows of items. However, there is one part where I believe I violate the prime directive of coding, which is not to repeat yourself: class Table_Creator(object): def __init__(self, given_num_of_items, max_num_of_items_per_row): self.total_num_of_items = range(given_num_of_items) self.max_num_of_items_per_row = max_num_of_items_per_row def create_grid(self): table = [] row = [] count = 0 while self.total_num_of_items: row.append(self.total_num_of_items.pop(0)) count += 1 if (not self.total_num_of_items) or (count == self.max_num_of_items_per_row): table.append(tuple(row)) row = [] count = 0 return table as you can see, I repeat the expressions row = [] and count = 0, and I would like to know if there is something I can do to avoid repetition in this case. First fix the indentation of the line starting with if. -- Bob Gailer 510-978-4454 Oakland, CA 919-636-4239 Chapel Hill, NC ___ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor
Re: [Tutor] don't repeat yourself; question about code optimization
[EMAIL PROTECTED] wrote: dear fellow Python enthusiasts: in the last year I have been experimenting with Python, and I set out to create a function that, given a number of items and a maximum number of items per row, would generate a table of rows of items. However, there is one part where I believe I violate the prime directive of coding, which is not to repeat yourself: class Table_Creator(object): def __init__(self, given_num_of_items, max_num_of_items_per_row): self.total_num_of_items = range(given_num_of_items) self.max_num_of_items_per_row = max_num_of_items_per_row def create_grid(self): table = [] row = [] count = 0 while self.total_num_of_items: row.append(self.total_num_of_items.pop(0)) count += 1 if (not self.total_num_of_items) or (count == self.max_num_of_items_per_row): table.append(tuple(row)) row = [] count = 0 return table as you can see, I repeat the expressions row = [] and count = 0, and I would like to know if there is something I can do to avoid repetition in this case. OK - to address your original question: def create_grid(self): table = [] while self.total_num_of_items: row = [] count = 0 while count self.max_num_of_items_per_row and self.total_num_of_items: row.append(self.total_num_of_items.pop(0)) count += 1 table.append(tuple(row)) return table -- Bob Gailer 510-978-4454 Oakland, CA 919-636-4239 Chapel Hill, NC ___ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor
Re: [Tutor] don't repeat yourself; question about code optimization
[EMAIL PROTECTED] wrote: dear fellow Python enthusiasts: in the last year I have been experimenting with Python, and I set out to create a function that, given a number of items and a maximum number of items per row, would generate a table of rows of items. However, there is one part where I believe I violate the prime directive of coding, which is not to repeat yourself: class Table_Creator(object): def __init__(self, given_num_of_items, max_num_of_items_per_row): self.total_num_of_items = range(given_num_of_items) self.max_num_of_items_per_row = max_num_of_items_per_row def create_grid(self): table = [] row = [] count = 0 while self.total_num_of_items: row.append(self.total_num_of_items.pop(0)) count += 1 if (not self.total_num_of_items) or (count == self.max_num_of_items_per_row): table.append(tuple(row)) row = [] count = 0 return table as you can see, I repeat the expressions row = [] and count = 0, and I would like to know if there is something I can do to avoid repetition in this case. Take advantage of slicing: def create_grid(self): table = [] for i in range(0, len(self.total_num_of_items), self.max_num_of_items_per_row): table.append(tuple(self.total_num_of_items[i : i + self.max_num_of_items_per_row])) return table -- Bob Gailer 510-978-4454 Oakland, CA 919-636-4239 Chapel Hill, NC ___ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor
