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)
-
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\$200_success– 200_success2016年12月04日 17:12:57 +00:00Commented Dec 4, 2016 at 17:12
-
\$\begingroup\$ Possible duplicate of Python code to weave rows \$\endgroup\$user124670– user1246702016年12月04日 22:48:54 +00:00Commented Dec 4, 2016 at 22:48
2 Answers 2
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 None
s, 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.
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 weave
methid.
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