Re: [Tutor] don't repeat yourself; question about code optimization CORRECTION

[Bob Gailer]

[EMAIL PROTECTED] wrote:
>
>
> 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))
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

[ALAN GAULD]

> 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

[Eric Brunson]

[EMAIL PROTECTED] wrote:
>
>
> 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)),

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

[tpc247]

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

[tpc247]

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 CORRECTION

[Bob Gailer]

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] 
> > 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

Re: [Tutor] don't repeat yourself; question about code optimization

[Bob Gailer]

[EMAIL PROTECTED] wrote:
>
>
> 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.
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

[Alan Gauld]

<[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

[tpc247]

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

[Bob Gailer]

[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

Re: [Tutor] don't repeat yourself; question about code optimization

[Bob Gailer]

[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

[Bob Gailer]

[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