Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

#Review:

Review:

#My solution:

My solution:

#Code:

Code:

#Review:

#My solution:

#Code:

Review:

My solution:

Code:

Removed an extra print statement
Source Link
mleyfman
  • 5.3k
  • 1
  • 25
  • 48
def find_move(original, result):
 """Returns the move the changes original into result, if possible"""
 if len(original) != len(result):
 raise ValueError("lists have different lengths")
 for counter, (a, b) in enumerate(zip(original, result)):
 if a != b:
 original_copy = original[counter:]
 try:
 original_copy.remove(b)
 except ValueError:
 raise ValueError("result has different values from original")
 if original_copy == result[counter+1:]:
 return find_forward_move(original, result, counter)
 else:
 # move is backwards
 a, b = find_forward_move(result, original, counter)
 return (b, a)
def find_forward_move(original, result, diff_start):
 """Produces the move that transforms original into result if moving forwards"""
 diff_end = 0
 for counter, (a, b) in enumerate(zip(original[diff_start+1:], result[diff_start+1:])):
 if a == b:
 print(counter + diff_start)
 diff_end = counter + diff_start
 break
 if diff_end == 0:
 diff_end = len(original) - 1
 # Test to make sure moving does the trick to ensure it really is 1 move only
 original_copy = original[:]
 original_copy.remove(original[diff_end])
 original_copy.insert(diff_start, original[diff_end])
 if original_copy != result:
 raise ValueError("More than 1 move detected")
 
 return diff_end, diff_start
def find_move(original, result):
 """Returns the move the changes original into result, if possible"""
 if len(original) != len(result):
 raise ValueError("lists have different lengths")
 for counter, (a, b) in enumerate(zip(original, result)):
 if a != b:
 original_copy = original[counter:]
 try:
 original_copy.remove(b)
 except ValueError:
 raise ValueError("result has different values from original")
 if original_copy == result[counter+1:]:
 return find_forward_move(original, result, counter)
 else:
 # move is backwards
 a, b = find_forward_move(result, original, counter)
 return (b, a)
def find_forward_move(original, result, diff_start):
 """Produces the move that transforms original into result if moving forwards"""
 diff_end = 0
 for counter, (a, b) in enumerate(zip(original[diff_start+1:], result[diff_start+1:])):
 if a == b:
 print(counter + diff_start)
 diff_end = counter + diff_start
 break
 if diff_end == 0:
 diff_end = len(original) - 1
 # Test to make sure moving does the trick to ensure it really is 1 move only
 original_copy = original[:]
 original_copy.remove(original[diff_end])
 original_copy.insert(diff_start, original[diff_end])
 if original_copy != result:
 raise ValueError("More than 1 move detected")
 
 return diff_end, diff_start
def find_move(original, result):
 """Returns the move the changes original into result, if possible"""
 if len(original) != len(result):
 raise ValueError("lists have different lengths")
 for counter, (a, b) in enumerate(zip(original, result)):
 if a != b:
 original_copy = original[counter:]
 try:
 original_copy.remove(b)
 except ValueError:
 raise ValueError("result has different values from original")
 if original_copy == result[counter+1:]:
 return find_forward_move(original, result, counter)
 else:
 # move is backwards
 a, b = find_forward_move(result, original, counter)
 return (b, a)
def find_forward_move(original, result, diff_start):
 """Produces the move that transforms original into result if moving forwards"""
 diff_end = 0
 for counter, (a, b) in enumerate(zip(original[diff_start+1:], result[diff_start+1:])):
 if a == b:
 diff_end = counter + diff_start
 break
 if diff_end == 0:
 diff_end = len(original) - 1
 # Test to make sure moving does the trick to ensure it really is 1 move only
 original_copy = original[:]
 original_copy.remove(original[diff_end])
 original_copy.insert(diff_start, original[diff_end])
 if original_copy != result:
 raise ValueError("More than 1 move detected")
 
 return diff_end, diff_start
added 898 characters in body
Source Link
mleyfman
  • 5.3k
  • 1
  • 25
  • 48

Ok, finally after some testing,#Review:

I would make passing in results that have an answerdifferent values, lengths, and multiple moves to be ValueErrors instead of None returns, as those don't seem to be valid input.

You missed a test case: your test cases do not cover multiple moves (which is supposed to fail by your definition).

You do not follow the Python programming style-guide: PEP-8 . Function names should be lowercase_with_underscores, docstrings are missing, spacing isn't consistent...

Additionally, defining functions within functions is generally less readable and less useful than defining them externally (so that worksthey might be used elsewhere for instance)

Lastly, I would have your tests run in some sort of main function. This can be done by adding this code snippet to the location of the tests:

if __name__ == '__main__':
 # run tests here

This allows you to run all the giventests automatically when you run the module with the tests, which simplifies testing.

#My solution:

The basic idea is to iterate through the values to find a difference, determine if it is a forward move or a backwards move, and then find where the move came from.

The enumerate function I'm using is basically adds a counter over the tuples for the zip function, so that I don't have to keep track of an extra variable and the results are in a convenient index, tuple format.

Additionally, I made different values, lengths, and multiple moves be ValueErrors as those don't seem to be valid input.

Note: You missed a test case: your test cases do not cover multiple moves (which is supposed to fail by your definition).

def findMovefind_move(original, result):
 """Returns the move the changes original into result, if possible"""
 if len(original) != len(result):
 raise ValueError("lists have different lengths")
 for counter, (a, b) in enumerate(zip(original, result)):
 if a != b:
 original_copy = original[counter:]
 try:
 original_copy.remove(b)
 except ValueError:
 raise ValueError("result has different values from original")
 if original_copy == result[counter+1:]:
 return findForwardMovefind_forward_move(original, result, counter)
 else:
 # move is backwards
 a, b = findForwardMovefind_forward_move(result, original, counter)
 return (b, a)
def findForwardMovefind_forward_move(original, result, diff_start):
 """Produces the move that transforms original into result if moving forwards"""
 diff_end = 0
 for counter, (a, b) in enumerate(zip(original[diff_start+1:], result[diff_start+1:])):
 if a == b:
 print(counter + diff_start)
 diff_end = counter + diff_start
 break
 if diff_end == 0:
 diff_end = len(original) - 1
 # Test to make sure moving does the trick to ensure it really is 1 move only
 original_copy = original[:]
 original_copy.remove(original[diff_end])
 original_copy.insert(diff_start, original[diff_end])
 if original_copy != result:
 raise ValueError("More than 1 move detected")
 
 return diff_end, diff_start

Ok, finally after some testing, I have an answer that works for all the given tests.

The basic idea is to iterate through the values to find a difference, determine if it is a forward move or a backwards move, and then find where the move came from.

The enumerate function I'm using is basically adds a counter over the tuples for the zip function, so that I don't have to keep track of an extra variable and the results are in a convenient index, tuple format.

Additionally, I made different values, lengths, and multiple moves be ValueErrors as those don't seem to be valid input.

Note: You missed a test case: your test cases do not cover multiple moves (which is supposed to fail by your definition).

def findMove(original, result):
 if len(original) != len(result):
 raise ValueError("lists have different lengths")
 for counter, (a, b) in enumerate(zip(original, result)):
 if a != b:
 original_copy = original[counter:]
 try:
 original_copy.remove(b)
 except ValueError:
 raise ValueError("result has different values from original")
 if original_copy == result[counter+1:]:
 return findForwardMove(original, result, counter)
 else:
 # move is backwards
 a, b = findForwardMove(result, original, counter)
 return (b, a)
def findForwardMove(original, result, diff_start):
 diff_end = 0
 for counter, (a, b) in enumerate(zip(original[diff_start+1:], result[diff_start+1:])):
 if a == b:
 print(counter + diff_start)
 diff_end = counter + diff_start
 break
 if diff_end == 0:
 diff_end = len(original) - 1
 # Test to make sure moving does the trick to ensure it really is 1 move only
 original_copy = original[:]
 original_copy.remove(original[diff_end])
 original_copy.insert(diff_start, original[diff_end])
 if original_copy != result:
 raise ValueError("More than 1 move detected")
 
 return diff_end, diff_start

#Review:

I would make passing in results that have different values, lengths, and multiple moves to be ValueErrors instead of None returns, as those don't seem to be valid input.

You missed a test case: your test cases do not cover multiple moves (which is supposed to fail by your definition).

You do not follow the Python programming style-guide: PEP-8 . Function names should be lowercase_with_underscores, docstrings are missing, spacing isn't consistent...

Additionally, defining functions within functions is generally less readable and less useful than defining them externally (so that they might be used elsewhere for instance)

Lastly, I would have your tests run in some sort of main function. This can be done by adding this code snippet to the location of the tests:

if __name__ == '__main__':
 # run tests here

This allows you to run all the tests automatically when you run the module with the tests, which simplifies testing.

#My solution:

The basic idea is to iterate through the values to find a difference, determine if it is a forward move or a backwards move, and then find where the move came from.

The enumerate function I'm using is basically adds a counter over the tuples for the zip function, so that I don't have to keep track of an extra variable and the results are in a convenient index, tuple format.

def find_move(original, result):
 """Returns the move the changes original into result, if possible"""
 if len(original) != len(result):
 raise ValueError("lists have different lengths")
 for counter, (a, b) in enumerate(zip(original, result)):
 if a != b:
 original_copy = original[counter:]
 try:
 original_copy.remove(b)
 except ValueError:
 raise ValueError("result has different values from original")
 if original_copy == result[counter+1:]:
 return find_forward_move(original, result, counter)
 else:
 # move is backwards
 a, b = find_forward_move(result, original, counter)
 return (b, a)
def find_forward_move(original, result, diff_start):
 """Produces the move that transforms original into result if moving forwards"""
 diff_end = 0
 for counter, (a, b) in enumerate(zip(original[diff_start+1:], result[diff_start+1:])):
 if a == b:
 print(counter + diff_start)
 diff_end = counter + diff_start
 break
 if diff_end == 0:
 diff_end = len(original) - 1
 # Test to make sure moving does the trick to ensure it really is 1 move only
 original_copy = original[:]
 original_copy.remove(original[diff_end])
 original_copy.insert(diff_start, original[diff_end])
 if original_copy != result:
 raise ValueError("More than 1 move detected")
 
 return diff_end, diff_start
added 422 characters in body
Source Link
mleyfman
  • 5.3k
  • 1
  • 25
  • 48
Loading
added 116 characters in body
Source Link
mleyfman
  • 5.3k
  • 1
  • 25
  • 48
Loading
added 309 characters in body
Source Link
mleyfman
  • 5.3k
  • 1
  • 25
  • 48
Loading
added 30 characters in body
Source Link
mleyfman
  • 5.3k
  • 1
  • 25
  • 48
Loading
added 1155 characters in body
Source Link
mleyfman
  • 5.3k
  • 1
  • 25
  • 48
Loading
Source Link
mleyfman
  • 5.3k
  • 1
  • 25
  • 48
Loading
lang-py

AltStyle によって変換されたページ (->オリジナル) /