8
Guido van Rossum
Last seen 8 years ago
Member for 11 years, 1 month, 2 days
Difficulty Normal
table[index(x, y)] = v
PS. I'm not going to verify that this actually works; I'd get a
headache. I trust that this is what Wikipedia told you to do. :-)
More
for x, s in enumerate(stones, 1):
Your choice to start x at 1 seems odd -- in the code below, "x - 1"
occurs twice while "x" alone occurs only once.
More
return total * len(stones) / 2
See note about constants and big Oh notation below.
More
return x + x_size * y
It's clear that you really want a two-dimensional list; then you could
just write table[x][y] instead of table[index(x, y)]. I suppse you've
been badly bitten by the bug in this piece of code for creating such a
table:
# DO NOT USE THIS
More
diff = abs(2 * part - total)
Style nit: Contrary to what many people seem to believe PEP 8
specifies, I'd like it when whitespace reflects the binding strength
of the operators:
abs(2*part - total)
More
best = None
If you initialize best to a large enough value you won't need to
special-case "best is None" below. It turns out 'total' is the
perfect "large enough" value, and it produces the right answer if the
list is empty. (Thinking about extreme cases often helps understand
the
More
I love that you're defining a class here that can be reused for other similar problems.
Why the renaming of x/y to u/v in __init__()? (Why not just name the arguments u and v as well?)
Isn't there some normalization you can apply so that comparing becomes simpler? (Sorry, I've forgotten all my
More
for s in itertools.chain.from_iterable(itertools.combinations(stones, r) for r in range(1, max_size + 1)):
This way of writing two nested loop as one using
itertools.chain.from_iterable() feels a bit confusing. (I actually
had to look it up!) Since you don't need to break out of the
More
All in all this code is pretty good.
import math
import itertools
import heapq
class Field(object):
def __init__(self, field):
Would be nice if the argument name was field_map (matching the call).
self._w = len(field[0])
self._h = len
More
def f(self):
Another bad choice of name. What's the point of writing a program if it's
hard to read? (I know, if you just want the solution, it doesn't matter.
But if you want to brag about your code, it does. :)
More
newState[newPos[0]][newPos[1]] = 0
I'd use a tuple of tuples to represent states (as I said earlier) and write
a helper function to compute a new state with a position replaced. The call
could be something like
new_state = state_replace(new_state, empty_row, empty_col,
node_st
More
newState[newPos[0]][newPos[1]] = 0
euclidianCost(newState, END), dir)]
More
#add the start node
Node(puzzle,puzzle,0,euclidianCost(puzzle,END), "")}
More
closedSet = {}
Why are these called sets when they are dicts? And why are they called
"open" and "closed"? (Maybe because that's what the Wikipedia page calls
them? That would explain several of your other poor naming choices as well.
:-)
After tracing through the code with pdb for a whil
More
for node in adj:
str(node.state) in openSet.keys() and openSet[str(node.state)].f() <
node.f():
Nit: to check whether a dict has a certain key, just write "if key in dict"
rather than "if key in dict.keys()".
More
continue
Why use 'continue' here? You could just as well revers the condition in the
if:
if not (...original condition...):
...add node...
I bet if you then use the distributive law on the condition it'll become
even clearer.
More
1]}
You really should be using tuples instead of lists for the coordinates.
More
bestF = bestNode.f()
Using a new feature of min(), you can write this entire function as follows:
return min(openSet.values(), key=lambda node: node.f())
This will call f() the minimal number of times. It may also make sense to
turn f() -- by whatever name -- into a module-le
More
def __init__(self, state, previous, g, h, dir):
Wow. Very poor choice of names for 'g' and 'h'.
More