Adding Items to a List<> of Objects Results in Duplicate Objects When Using a New in a Loop

Why does my ArrayList contain N copies of the last item added to the list?

This problem has two typical causes:

  • Static fields used by the objects you stored in the list

  • Accidentally adding the same object to the list

Static Fields

If the objects in your list store data in static fields, each object in your list will appear to be the same because they hold the same values. Consider the class below:

public class Foo {
private static int value;
// ^^^^^^------------ - Here's the problem!

public Foo(int value) {
this.value = value;
}

public int getValue() {
return value;
}
}

In that example, there is only one int value which is shared between all instances of Foo because it is declared static. (See "Understanding Class Members" tutorial.)

If you add multiple Foo objects to a list using the code below, each instance will return 3 from a call to getValue():

for (int i = 0; i < 4; i++) {      
list.add(new Foo(i));
}

The solution is simple - don't use the static keywords for fields in your class unless you actually want the values shared between every instance of that class.

Adding the Same Object

If you add a temporary variable to a list, you must create a new instance of the object you are adding, each time you loop. Consider the following erroneous code snippet:

List<Foo> list = new ArrayList<Foo>();    
Foo tmp = new Foo();

for (int i = 0; i < 3; i++) {
tmp.setValue(i);
list.add(tmp);
}

Here, the tmp object was constructed outside the loop. As a result, the same object instance is being added to the list three times. The instance will hold the value 2, because that was the value passed during the last call to setValue().

To fix this, just move the object construction inside the loop:

List<Foo> list = new ArrayList<Foo>();        

for (int i = 0; i < 3; i++) {
Foo tmp = new Foo(); // <-- fresh instance!
tmp.setValue(i);
list.add(tmp);
}

Remove duplicates from a list of objects based on property in Java 8

You can get a stream from the List and put in in the TreeSet from which you provide a custom comparator that compares id uniquely.

Then if you really need a list you can put then back this collection into an ArrayList.

import static java.util.Comparator.comparingInt;
import static java.util.stream.Collectors.collectingAndThen;
import static java.util.stream.Collectors.toCollection;

...
List<Employee> unique = employee.stream()
.collect(collectingAndThen(toCollection(() -> new TreeSet<>(comparingInt(Employee::getId))),
ArrayList::new));

Given the example:

List<Employee> employee = Arrays.asList(new Employee(1, "John"), new Employee(1, "Bob"), new Employee(2, "Alice"));

It will output:

[Employee{id=1, name='John'}, Employee{id=2, name='Alice'}]

Another idea could be to use a wrapper that wraps an employee and have the equals and hashcode method based with its id:

class WrapperEmployee {
private Employee e;

public WrapperEmployee(Employee e) {
this.e = e;
}

public Employee unwrap() {
return this.e;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
WrapperEmployee that = (WrapperEmployee) o;
return Objects.equals(e.getId(), that.e.getId());
}

@Override
public int hashCode() {
return Objects.hash(e.getId());
}
}

Then you wrap each instance, call distinct(), unwrap them and collect the result in a list.

List<Employee> unique = employee.stream()
.map(WrapperEmployee::new)
.distinct()
.map(WrapperEmployee::unwrap)
.collect(Collectors.toList());

In fact, I think you can make this wrapper generic by providing a function that will do the comparison:

public class Wrapper<T, U> {
private T t;
private Function<T, U> equalityFunction;

public Wrapper(T t, Function<T, U> equalityFunction) {
this.t = t;
this.equalityFunction = equalityFunction;
}

public T unwrap() {
return this.t;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
@SuppressWarnings("unchecked")
Wrapper<T, U> that = (Wrapper<T, U>) o;
return Objects.equals(equalityFunction.apply(this.t), that.equalityFunction.apply(that.t));
}

@Override
public int hashCode() {
return Objects.hash(equalityFunction.apply(this.t));
}
}

and the mapping will be:

.map(e -> new Wrapper<>(e, Employee::getId))

List append() in for loop raises exception: 'NoneType' object has no attribute 'append'

The list.append function does not return any value(but None), it just adds the value to the list you are using to call that method.

In the first loop round you will assign None (because the no-return of append) to a, then in the second round it will try to call a.append, as a is None it will raise the Exception you are seeing

You just need to change it to:

a = []
for i in range(5):
# change a = a.append(i) to
a.append(i)
print(a)
# [0, 1, 2, 3, 4]

list.append is what is called a mutating or destructive method, i.e. it will destroy or mutate the previous object into a new one(or a new state).

If you would like to create a new list based in one list without destroying or mutating it you can do something like this:

a=['a', 'b', 'c']
result = a + ['d']

print result
# ['a', 'b', 'c', 'd']

print a
# ['a', 'b', 'c']

As a corollary only, you can mimic the append method by doing the following:

a = ['a', 'b', 'c']
a = a + ['d']

print a
# ['a', 'b', 'c', 'd']

Python append object elements to list in for loop, get duplicate element?

You already got the solution in khelwood's comment - as a more general answer, callables (classes, functions, methods etc) are not called if you don't explicitely apply the call operator, IOW the parens.

As a side note: you would certainly benefit from passing all values directly to you Video class initializer, and restricting your SQL query to the fields you actually use:

class Video:
def __init__(self, id, title, time, category, url):
self.id = id
self.title = title
self.time = time
self.category = category
self.url = url

def getVideoInfo():
# I assume the db fields have the same names as
# the class attributes - else fix this with the
# right field names
sql = "SELECT id, title, timen category, url FROM VIDEO"
ls = dbUtil.get_data_tuple_ls(sql)
video_ls = [Video(*row) for row in ls]
return video_ls

Remove duplicates from a List T in C#

Perhaps you should consider using a HashSet.

From the MSDN link:

using System;
using System.Collections.Generic;

class Program
{
static void Main()
{
HashSet<int> evenNumbers = new HashSet<int>();
HashSet<int> oddNumbers = new HashSet<int>();

for (int i = 0; i < 5; i++)
{
// Populate numbers with just even numbers.
evenNumbers.Add(i * 2);

// Populate oddNumbers with just odd numbers.
oddNumbers.Add((i * 2) + 1);
}

Console.Write("evenNumbers contains {0} elements: ", evenNumbers.Count);
DisplaySet(evenNumbers);

Console.Write("oddNumbers contains {0} elements: ", oddNumbers.Count);
DisplaySet(oddNumbers);

// Create a new HashSet populated with even numbers.
HashSet<int> numbers = new HashSet<int>(evenNumbers);
Console.WriteLine("numbers UnionWith oddNumbers...");
numbers.UnionWith(oddNumbers);

Console.Write("numbers contains {0} elements: ", numbers.Count);
DisplaySet(numbers);
}

private static void DisplaySet(HashSet<int> set)
{
Console.Write("{");
foreach (int i in set)
{
Console.Write(" {0}", i);
}
Console.WriteLine(" }");
}
}

/* This example produces output similar to the following:
* evenNumbers contains 5 elements: { 0 2 4 6 8 }
* oddNumbers contains 5 elements: { 1 3 5 7 9 }
* numbers UnionWith oddNumbers...
* numbers contains 10 elements: { 0 2 4 6 8 1 3 5 7 9 }
*/

Why list item is update it's value base on current object that is already added?

MessageModel is a class - a reference type. When you add it to the list you are adding a reference to the object which is the same as the reference stored in the Message variable.

There is effectively only one object and both the message variable and the list are pointing at it. If you want to change the name of the Message variable and not affect the list you will have to first create a new object and assign it to Message:

MessageModel Message = new MessageModel();
Message.Name = "MyName";
BindingList<MessageModel> list = new BindingList<MessageModel>();
list.Add(Message); // list[0].Name = MyName
Message = new MessageModel();
Message.Name = "New Name"; //list[0].Name = MyName


Related Topics



Leave a reply



Submit