0

I a wrote a function SwapCities that is able to swap entries 3 and 4 in a list.

So f.e. [0,1,2,3,4] should become [0,1,2,4,3]. This function works perfectly, but strangely my original list also changes which I do not want.

This is my code:

def SwapCities(solution):
    n = 3##randint(0,NumberOfCities-1)
    m = 4##randint(0,NumberOfCities-1)
    result = solution
    temp1 = solution[n]
    temp2 = solution[m]
    result[n] = temp2
    result[m] = temp1
    return result

print "Start"
IncumbentSolution = list(x for x in range(0,NumberOfCities))
print IncumbentSolution

print "After swap" NewSolution = SwapCities(IncumbentSolution)
print NewSolution

print "Original solution"
print IncumbentSolution

I get the following result:

How many cities?
8 Start [0, 1, 2, 3, 4, 5, 6, 7]
After swap [0, 1, 2, 4, 3, 5, 6, 7]
Original solution [0, 1, 2, 4, 3, 5, 6, 7]   (why did this change?!)

As you can see my original solution changed which it should not do.

I have no clue why this happens. Even when I change the code such that the changes to are applied to a copy of the original list I get this result. Could someone explain what I am doing wrong?

IncumbentSolution = list(x for x in range(0,NumberOfCities))
print "Start"
print IncumbentSolution

print "After swap"
tmpsolution = IncumbentSolution
NewSolution = SwapCities(tmpsolution)
print NewSolution

print "Original solution"
print IncumbentSolution
Stefano Sanfilippo
  • 32,265
  • 7
  • 79
  • 80
  • About copying lists: http://stackoverflow.com/a/184660/577423 – Howard May 18 '13 at 15:42
  • 3
    It's worth noting you don't need temporary variables to do a swap in Python - `solution[n], solution[m] = solution[m], solution[n]`. This is shorter, more readable and also more efficient. – Gareth Latty May 18 '13 at 15:48

2 Answers2

7

SwapCities is mutating the contents of solution. Since solution points to the same list as IncumbentSolution, the values inside IncumbentSolution are altered too.


To preserve the original values in IncumbentSolution, make a new copy of the list:

tmpsolution = list(IncumbentSolution)

makes a shallow copy of the the original list. Since the contents of IncumbentSolution are immutable numbers, a shallow copy suffices. If the contents included, say, dicts which were also being mutated, then you would need to make a deep copy of the list:

import copy
tmpsolution = copy.deepcopy(IncumbentSolution)
unutbu
  • 842,883
  • 184
  • 1,785
  • 1,677
  • 1
    I'd always recommend using `list(IncumbentSolution)`. It does the same job, but is more readable. `[:]` is generally confusing to new Python users (see how many questions there are about it on SO alone), and while that isn't always a good reason not to use something, it is when we have a much better replacement at hand. – Gareth Latty May 18 '13 at 15:46
  • @Lattyware: Good idea. It is confusing to use `arr[:]` especially when using `NumPy` since there it returns a view, not a copy. – unutbu May 18 '13 at 19:57
1

That's because you modified the list inside the function.

By passing the list to the function you simply created another reference to the same object, solution and IncumbentSolution actually point to the same list object.

You should pass a shallow copy to the function, using IncumbentSolution[:].

>>> def func(x):
...     return id(x)
... 
>>> lis = range(5)
>>> id(lis),func(lis)     #both `x` and `lis` point to the same object
(163001004, 163001004)

>>> id(lis),func(lis[:])  #pass a shallow copy, `lis[:]` or `list(lis)`
(163001004, 161089068)
Ashwini Chaudhary
  • 244,495
  • 58
  • 464
  • 504