3
\$\begingroup\$

I want to write a cleaner version of this code. It's about two different rows consisting of numbers that have to be weaved. I have already asked this question at this link, but I have translated my code in English so that it's maybe easier to help me improve it.

It's for an assignment for a course I'm currently following:

Traveler, if riches are to be achieved, the solution only has to be weaved. 
5,4 4,5 8,7
Weave 6,3 3,2 9,6 4,3
Weave 7,6
Weave 9,8
Weave 5,5 7,8 6,5 6,4

I have to start with the coordinates on the first row, weave the coordinates of the second row through the first row, then weave the coordinates of the third row through the resulting row etc. Note, if two rows of unequal length have to be woven, the two rows are woven as far as possible. After the elements of the shortest row have been exhausted, the row is completed by adding the remaining elements of the longest row.

The input is this (my code already returns the desired output):

5,4 4,5 8,7=6,3 3,2 9,6 4,3=7,6=9,8=5,5 7,8 6,5 6,4

The output has to be like this:

6,4
6,5
10,8
8,8
8,6
7,5
7,3
7,4
5,5
4,2
9,7
10,6
5,3

Coordinate.py

class Coordinate:
 def __init__(self, x_coord, y_coord):
 self.x_coord = x_coord
 self.y_coord = y_coord
 def calculate_new_coord(self):
 self.x_coord += 1
 self.y_coord += 0
 return self.x_coord, self.y_coord

CoordinateRow.py

class CoordinateRow:
 def __init__(self, row):
 self.row = row
 def append(self, coord):
 self.row += [coord]
 def extend(self, row):
 for coordinate in row:
 self.row += [coordinate]
 def weave(self, weaveRow):
 result = CoordinateRow([])
 for i in range(len(self.row)):
 if i < len(self.row):
 result.append(self.row[i])
 if i < len(weaveRow.row):
 result.append(weaveRow.row[i])
 if len(weaveRow.row) > len(self.row): #Als 2e rij(te weaven door 1e rij) groter dan 1e rij(bestaande rij)
 result.extend(weaveRow.row[len(self.row):]) #voeg het verschil toe aan result 
 return result

Excerpt of the main code

file_input = file.read()
strip_file = file_input.strip()
splits_file = strip_file.split("=")
from Coordinate import Coordinate
from CoordinateRow import CoordinateRow
def get_weaved_rows(splits_bestand):
 rows = []
 for i in range(len(splits_bestand)):
 rows += [CoordinateRow(splits_bestand[i].split())] 
 previous_row = rows[0]
 for row in rows[1:]:
 previous_row = previous_row.weave(row)
 return previous_row
def get_new_coordinates(weaved_rows):
 for coordinate in weaved_rows.row:
 current_coordinates = Coordinate(int(coordinate[0]), int(coordinate[2])) #coordinaat x[0] en y[2] worden int(x,y)
 new_coordinates = current_coordinates.calculate_new_coord() #coordinaten (x,y) na verwerking 
 print '%d,%d' %(new_coordinates[0], new_coordinates[1])
weaved_rows = get_weaved_rows(splits_file)
get_new_coordinates(weaved_rows)
AlexV
7,3532 gold badges24 silver badges47 bronze badges
asked Dec 4, 2016 at 14:03
\$\endgroup\$
2
  • 1
    \$\begingroup\$ I gave you a succinct solution in response to your previous question. Even if you didn't like the fact that the solution was based on functional programming rather than OOP, you could still have made an effort to incorporate some of those suggestions instead of ignoring them entirely. \$\endgroup\$ Commented Dec 4, 2016 at 17:12
  • \$\begingroup\$ Possible duplicate of Python code to weave rows \$\endgroup\$ Commented Dec 4, 2016 at 22:48

2 Answers 2

2
\$\begingroup\$

There are some things, which can be improved.

For one, the weave method can be rewritten using itertools, see this SO post for examples. The idea is to combine elements via iterators (with function similar to zip - see izip_longest), chain and filter out Nones, which were possibly added from missing counterparts.

Then, this pattern:

rows = []
for i in range(len(splits_bestand)):
 rows += [CoordinateRow(splits_bestand[i].split())] 

can be replaced with more compact comprehension:

rows = [CoordinateRow(sb.split()) for sb in splits_bestand]

Also, Python list has extend method, which can be used directly in .extend method, like: self.row.extend(row). Similarly for append method. If you will need those at all after using itertools.

Code for previous raw can also be easily rewritten with itertools. Left as an exercise.

answered Dec 4, 2016 at 14:32
\$\endgroup\$
1
\$\begingroup\$

As @RomanSusi already mentioned in his answer, lists already have an append and extend method. What he did not say, though, is that this means that your class CoordinateRow could just be an extension of the built-in list with an added weavemethid.

Sub-classing list has the added advantage that you get len, empty constructor, printing of content and indexing for free. You don't even need to define the constructor, since it is not changed with respect to the list one:

class CoordinateRow(list):
 def weave(self, weaveRow):
 result = CoordinateRow()
 for i in range(len(self)):
 if i < len(self):
 result.append(self[i])
 if i < len(weaveRow):
 result.append(weaveRow[i])
 if len(weaveRow) > len(self):
 result.extend(weaveRow[len(self):])
 return result
answered Dec 4, 2016 at 15:00
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.