Skip to main content
replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

Python is passing something like a reference, so the changes you make to potential_ans will persist, at least when you do things like appending letters. Read more on this from this SO postSO post.

Python is passing something like a reference, so the changes you make to potential_ans will persist, at least when you do things like appending letters. Read more on this from this SO post.

Python is passing something like a reference, so the changes you make to potential_ans will persist, at least when you do things like appending letters. Read more on this from this SO post.

Source Link
sunny
  • 1.9k
  • 1
  • 13
  • 29

Proviso I have not checked the logic of your code, mostly just the styling and language features you could have used but missed.

Make your functions single use

This is a general rule of programming, but one I have found especially good to enforce in Python, particularly because lots of small, single-use functions just look so beautiful when you code them up in PEP-style (which I am far from good at myself). So I'm not sure why you're printing in transform. Generally, you should try to keep logging as its own function, separate from calculations and the like. This is especially true in your function where you print the result and then return it. Why not return and then print wherever you return from, to make your code easier to maintain and understand?

It's easy to avoid globals in Python

You mentioned that you didn't want to use global variables. You don't have to. You can pass a variable to a function whenever you find yourself worrying about manipulating globals. You don't want your function in any way to depend on the name of a global if you can avoid it. There are many ways to fix this. One is to wrap your global in some kind of sensible class object. Additionally, and complementary, you should pass everything into your function as a parameter, even if it is a global, so you can do something like this:

def transform(english_words, start, end, count, potential_ans):

Python is passing something like a reference, so the changes you make to potential_ans will persist, at least when you do things like appending letters. Read more on this from this SO post.

Efficiency

Seems like you go through your english_words more than you need to. Why don't you break out of your for loop once you find the match you need?

Also on efficiency, I'd recommend doing the start==end check after the count check.

You can eliminate the if count == 0 step altogether if you make use of a nice feature of Python that [] is a false value. Then you can initially pass in an empty list and build it up with your recursive calls. Once you're passing potential_ans as a parameter, initially pass in an empty list and then use this statement:

potential_ans = potential_ans or [start]

This might be more efficient (easy to test) and it's certainly clearer and more succinct than the count check you are using now.

Confusing parts of your code

What does count do in transform? I see you increment it sometimes in transform and you also use it in is_diff_one. As a reader of your code, I wish you'd comment on whether there counts are related...is this some funky global variable manipulation or just your favorite count variable name? This is exactly where comments can save your reader a lot of work.

Actually, why do you set up potential_ans as a global variable when you redefine it inside your function anyway? Why don't you just initialize it yourself and keep passing it as a parameter (see above). This is a design decision, so you should comment here to explain to your otherwise puzzled reader.

What is the point of your last expression in your for loop over english_words, this one: potential_ans[:-1]. This one has really got me puzzled. Are you changing something? So far as I know Python syntax, you're simply referring to the last element of this list but not doing anything to/with it. Am I missing something? If so, a comment would help here. If not, this should be deleted as it doesn't affect the program at all apart from running an unnecessary expression.