I have two lists of classes:
MyClass1 {
String var1;
String var2;
String var3;
}
MyClass2 {
String var1;
String var2;
String var3;
}
Each of them contains:
MyClass1 = [{"one", "3", "6"},{"two", "6", "1"}]
MyClass2 = [{"one", "7", "7"}, {"four", "2", "9"}]
I want to obtain as result:
MyClass1 = [{"one", "7", "7"}, {"two", "6", "1"}, {"four", "2", "9"}]
So, I implemented this method:
public void mergeListWithRemotely(List<MyClass1> list, List<MyClass2> remotedList)
{
try {
while (remotedList.iterator().hasNext()) {
AtomicInteger position = new AtomicInteger(-1);
MyClass1 el = (MyClass1) remotedList.iterator().next();
Optional<MyClass1> e = list.stream()
.peek(x -> position.incrementAndGet())
.filter(x -> x.getVar1() == el.getVar1())
.findFirst();
if (e.isPresent())
list.set(position.get(), el);
else
list.add(el);
}
} catch (Exception e) {
e.printStackTrace();
}
}
When I execute this function, it doesn't look performant because it seems hanging even it doesn't catch any error. Is there a possible way to improve performance to complete the merge and obtain result?
There a bunch of misconceptions and mistakes here; your problem isn't just one thing.
list.iterator()
makes an iterator. A new one. Every time you invoke it.
This:
List list = List.of("Hello");
while (list.iterator().hasNext()) {
list.iterator().next();
}
Runs forever. list.iterator()
makes a new iterator every time you call it (which will iterate a single element - Hello
), and this new iterator does have a next element.
What you wanted is this:
List list = List.of("Hello");
var it = list.iterator();
while (it.hasNext()) {
it.next();
}
This exits near instantly; that while loop runs once and exits. Because we only call list.iterator()
once, whereas in the snippet before, we call it every time we loop.
You've made things needlessly complicated. You can just do:
for (var elem : remotedList) {
doStuffWith(elem);
}
Instead of:
var it = remotedList.iterator();
while (it.hasNext()) {
var elem = it.next();
doStuffWith(elem);
}
You're 'casting' a Class1 instance to a Class2 instance. This is not possible. At all. Nothing in java allows you to do this.
Yes, in your example, Class1
and Class2
are structurally identical; they have the exact same fields. Same types, same names. Nevertheless, a Class1 instance is not a Class2 instance. A cast operation just asserts a type - it does nothing if the thing you are casting is indeed what you assert it is, other than tell the compiler that it is this thing, and throws a ClassCastException if the thing you aren't casting isn't of the right type. It does not convert. Ever..
Unfortunately, the cast operator is used for 3 pretty much completely unrelated things. Just like 5 + 2
is 7, but "Hello" + "World"
is "HelloWorld" - two completely different things (String concatenation vs numeric addition) that so happen to both use the +
operator, the cast operator does 3 different things.
If the type you are casting to is a primitive, it converts:
double v = 5.5;
int x = (int) v; // converts 5.5 to 5.
If the type you are casting isn't a primitive, then it typechecks:
Object x = "5";
Integer y = (Integer) x; // fails
This does not make y
'5'. This fails: It checks if the thing x
is pointing at is an instance of integer. It is not, therefore, fails.
If you are casting generics, it does absolutely nothing at all, other than tell the compiler to just assume you are right and that the programmer accepts the problems that will occur if the assertion is incorrect. It's a "compiler, shut up, I know what I am doing, just compile it already" operator. It literally generates zero bytecode:
List list = new ArrayList();
list.add("Hello");
List<Integer> numbers = (List<Integer>) list;
the above code compiles and runs fine with no errors. However, if you attempt to interact with the numbers
list, you will get a ClassCastException
when you attempt to e.g. read an entry from it (Integer i = numbers.get(0);
for example), even though there is no cast on the line that throws.
You don't. Java does not have anything baked into it that lets you. And that is intentional. Imagine it did. You'd blow your friend's face off. Imagine you'd have these 2 types:
public class Camera {
Person target;
void shoot() [
// makes a picture of the target
}
}
public class Gun {
Person target;
void shoot() {
// kills the target
}
}
These 2 classes are structurally identical. Languages that engage in structural typing would let you treat a camera as a gun and vice versa. And thus, you'd blow your friend's face off. Java is not such a language. It simply has no way to convert a Camera to a Gun. Casts cannot do it, nor can anything else. You can, of course, write this:
void spyCraftMurder(Camera c) {
Gun gun = new Gun();
gun.setPerson(c.getPerson());
gun.shoot();
}
i.e. you manually write code that converts a Class1 instance to a Class2 instance, dutifully copying each and every field. You're free to do that. But, you have 2 classes that are structurally identical that you wish to treat as equivalent... that means you should be having just the one class and not both of them. The real place to fix your code isn't here, the mistake is already made: Why do you have 2 classes? You shouldn't be having this.
You want a.equals(b)
, not x.getVar1() == el.getVar1()
"Functional" style has benefits and drawbacks. You use the right tool. This usage of lambdas is disastrously bad, don't use it here. You have to futz about with AtomicIntegers just to count, and you have created a sequential requirement of sorts by having 2 related things (peeking and filtering), when generally the point of the functional stuff is that each element is independent. For example, if the 5th element is a match out of a list of 10 elements, then if this is done in parallel, you might end up with a count of 5, 6, 7, 8, 9, or 10, depending on the phase of the moon. This probably won't happen but the point of this functional stuff is that the collection types are free to optimize or not depending on their whim, and thus you've written broken code. You just wanted:
boolean replaced = false;
for (int i = 0; i < list.size(); i++) {
if (list.get(i).getVar1().equals(el.getVar1())) {
list.set(i, el);
replaced = true;
break;
}
}
if (!replaced) list.add(el);
Do not catch exceptions unless you either [A] handle them or [B] throw them onwards, possibly wrapped or modified to add relevant state.
Logging an exception is not handling it (except in rare circumstances).
You should just add throws
to your method and throw what you need. If you can't do that, for example because you're implementing an interface or extending a class and you can't, then the correct content of a catch block is:
catch (Exception iCannotBeBotheredToThingAboutThisRightNow) {
throw new RuntimeException("unhandled", iCannotBeBotheredToThingAboutThisRightNow);
}
Never e.printStackTrace();
. This causes any failure to result in 85 stack traces or so (because your code just keeps going and returns bogo results, your system is pretty much by definition in a state the programmer never considered, so of course more exceptions will occur then. If all exceptions are dealt with using the 'log it and continue' paradigm, then they all print a stack trace and continue the domino effect of errors.
For each element in list1, you check every element in list 2 - that means yur algorithm has a performance characteristic of O(n^2)
- which roughly means: A chart that charts 'size of input' against 'time taken to produce a result' looks like y = x^2
- that's not great: As input grows linearly, your time taken grows exponentially.
For example, if both lists are '10' large, you need 100 operations. Double the list sizes to 20 and now you need 400 operations. Make it lists of 10,000 each and you now need 100,000,000, which will actually take noticable time (about a second, probably less, but not quite instant anymore).
Your algorithm has the advantage that the input lists do not need to be sorted and no lookup code is required, and can 'resolve duplicates' regardless of where they are. But the downside is, well, the O(n^2)
characteristic.
You can fix this, but this requires either [A] that both lists are sorted and you go through in sorted fashion, or [B] an O(1)
or O(log n)
lookup system. For example, by using HashMap
. Your description is too vague to give any more pointers than this.
However, unless your input size are higher than 1000 elements, this isn't relevant, and any system can near instantly run this algorithm.