Return to Answer

• 1

• 228
• 1
• 5

Code:

It appears to me that your code style is just fine! I have only a few comments:

Whenever you see something like for i in range(len(mylist)), try to see if you can use a "for-in" loop. An example of this is your function _mutate(). The whole function can be rewritten as:

for citizen in self.population:
 if random.random() <= self.mutationRate:
 index1 = random.randint(0,len(self.route)-1)
 index2 = random.randint(0,len(self.route)-1)
 copy = citizen.route[index1]
 citizen.route[index1] = citizen.route[index2]
 citizen.route[index2] = copy

There is even possible to rewrite the flip stage as

citizen.route[i1], citizen.route[i2] = citizen.route[i2], citizen.route[i1]

Index is in my opinion self explanatory, so there is no need to use the whole index word.

Another issue I see is that it is hard to tell if some functions mutate (that means change) the state of the model given the functions name and description. For example _fitness(). This function changes the state of your class, but neither the function name nor the description explains this. Consider changing the name to _update_fitness() or something similar.

Comments:

Everyone disagrees when it comes to how to comment code. When that is said, there is some consensus. Self documenting code is the best. Try to write your code as clear as possible. If you can do this, then there is not need for to many comments.

I like that you use block comments on function and classes, but some of the are not necessary in my opinion. The rule here is basically to write code that explains why you do something. It appears to me that you have a tendency to explain the implementation, though the implementation can be seen just fine by the code. An example:

def _mutate(self):
 """
 _mutate iterates through the entire
 self.population. If a random.random()
 call returns a value <= self.mutationRate,
 then two random locations in a single 
 citizen's route are flipped.
 """
 for i in xrange(0,len(self.population)):
 if random.random() <= self.mutationRate:
 index1 = random.randint(0,len(self.route)-1)
 index2 = random.randint(0,len(self.route)-1)
 copy = self.population[i].route[index1]
 self.population[i].route[index1] = self.population[i].route[index2]
 self.population[i].route[index2] = copy

In my opinion, this code explains itself very well. I would look into another Stack Overflow answer as well as I feel it explains things well.

There are probably more stuff i could mention, an someone else might also disagree with the things I have pointed out, but at least you have someone elses opinion.

I also know this is for a school project. In a real life project, I would probable prefer less and clearer comments ;)