Datentypen NPE in längerer Methode

[Xyz1]

Hallo, also mal vorweg, es ist eine längere Methode geworden, welche mir jetzt Kopfweh bereitet. Code:

	LinkedHashMap<Long, Sys> systems = new LinkedHashMap<>();
	LinkedHashMap<Long, Sta> stations = new LinkedHashMap<>();
	LinkedHashMap<Long, Lis> listings = new LinkedHashMap<>();

	float ...;
	float ...;
	float ...;
	int ...;
	int ...;

// ...

	ArrayList<Lis[]> getAlgo2(App app) {
		ArrayList<Sys> sl = new ArrayList<>();
		for (Sys s : systems.values()) {
			if (getDisToSol(s) < maxDisToSol) {
				sl.add(s);
			}
		}
		ArrayList<Lis[]> results1 = new ArrayList<Lis[]>();
		ArrayList<Lis[]> results2 = new ArrayList<Lis[]>();

		getAlgo2(sl, results1, results2, new Lis[4], false, 0, 3, new HashSet<Long>());

		ArrayList<Lis[]> results = new ArrayList<>();
		results.addAll(results1);
		results.addAll(results2);
		return results;
	}

	//////////

	void getAlgo2(ArrayList<Sys> sl, ArrayList<Lis[]> results1, ArrayList<Lis[]> results2, Lis[] res, boolean planets,
			int index, int n, HashSet<Long> used) {
		if (index == n) {
			if (planets) {
				if (results1.isEmpty() || compareListings2(res, results1.get(results1.size() - 1)) < 0) {
					if (results1.size() == 100) {
						results1.remove(results1.size() - 1);
					}
					Lis[] l = new Lis[n + 1];
					System.arraycopy(res, 0, l, 0, n + 1);
					results1.add(l);
					Collections.sort(results1, (Lis[] l1, Lis[] l2) -> compareListings(l1, l2));
				}
			} else {
				if (results2.isEmpty() || compareListings2(res, results2.get(results2.size() - 1)) < 0) {
					if (results2.size() == 100) {
						results2.remove(results2.size() - 1);
					}
					Lis[] l = new Lis[n + 1];
					System.arraycopy(res, 0, l, 0, n + 1);
					results2.add(l);
					Collections.sort(results2, (Lis[] l1, Lis[] l2) -> compareListings(l1, l2));
				}
			}
			return;
		}
		if (index == 0) {
			for (Sys sys : sl) {
				if (!used.contains(sys.id)) {
					used.add(sys.id);
					for (Sta sta : sys.stations) {
						for (Lis lis : sta.listings) {
							if (lis.buy_price > 0 && lis.supply >= minSupDem) {
								res[index] = lis;
								if (sta.isPlanetary)
									getAlgo2(sl, results1, results2, res, true, index + 1, n, used);
								else
									getAlgo2(sl, results1, results2, res, planets, index + 1, n, used);
							}
						}
					}
					used.remove(sys.id);
				}
			}
		} else {
			Lis lis1 = res[index - 1];
			Sta sta1 = stations.get(lis1.station_id);
			Sys sys1 = systems.get(sta1.system_id);
			for (Sys sys : sl) {
				if (!used.contains(sys.id)) {
					used.add(sys.id);
					for (Sta sta : sys.stations) {
						Lis lis2 = sta.listingsMap.get(lis1.com_id);
						if (lis2 != null && lis1.buy_price > 0 && lis1.buy_price < lis2.sell_price
								&& lis1.supply >= minSupDem && lis2.demand >= minSupDem) {
							for (Lis lis : sta.listings) {
								if (lis.buy_price > 0 && lis.supply >= minSupDem) {
									res[index] = lis2;
									res[index + 1] = lis;
									if (sta.isPlanetary)
										getAlgo2(sl, results1, results2, res, true, index + 2, n, used);
									else
										getAlgo2(sl, results1, results2, res, planets, index + 2, n, used);
								}
							}
						}
					}
					used.remove(sys.id);
				}
			}
		}
	}

	//////////

	int compareListings2(Lis[] a, Lis[] b) {
		int sum1 = 0;
		for (int i = 0; i < a.length - 1; i += 2) {
			sum1 += a[i + 1].sell_price - a[i].buy_price;
		}
		int sum2 = 0;
		for (int i = 0; i < b.length - 1; i += 2) {
			sum2 += b[i + 1].sell_price - b[i].buy_price;
		}
		return Integer.compare(sum1, sum2);
	}

Ich rufe getAlgo2 auf... und die NPE tritt hier in Zeile 111 auf (sum1 += a[i + 1].sell_price - a[i].buy_price; , ne es ist jetzt Zufall, dass es gerad Zeile 111 ist).

Habt ihr eine Idee wieso? Wenn (index == n) dann sollte res eigentlich nicht leer sein.

 

[mrBrown]

Gibts das ganz auch in nicht-obfuskiert?

 

[Xyz1]

Gibts das ganz auch in nicht-obfuskiert?

Es sind knapp 1000 Zeilen und ich habe schon die essentiellen drei Methoden "extrahiert".

 

[Xyz1]

Wenn keiner herausfindet, weswegen die NPE in der Methode compareListings2() und getAlgo2() auftritt, also wo ich en Check vergessen hatte, dann kann das Thema wieder gelöscht werden.
Ich bedanke mich dennoch.

 

[Xyz1]

Fand gerade heraus, woran es lag; 4 sachen die indices betreffend waren falsch

	ArrayList<Lis[]> getAlgo2(App app) {
		int n = 5;

		ArrayList<Sys> sl = new ArrayList<>();
		for (Sys s : systems.values()) {
			if (getDisToSol(s) < maxDisToSol) {
				sl.add(s);
			}
		}
		ArrayList<Lis[]> results1 = new ArrayList<Lis[]>();
		ArrayList<Lis[]> results2 = new ArrayList<Lis[]>();

		getAlgo2(sl, results1, results2, new Lis[n], false, 0, n - 1, new HashSet<Long>()); // array with exact 5
																							// elements...

		ArrayList<Lis[]> results = new ArrayList<>();
		results.addAll(results1);
		results.addAll(results2);
		return results;
	}

	//////////

	void getAlgo2(ArrayList<Sys> sl, ArrayList<Lis[]> results1, ArrayList<Lis[]> results2, Lis[] res, boolean planets,
			int index, int n, HashSet<Long> used) {
		if (index > n) { // index must be 2 greater than n...
			if (planets) {
				if (results1.isEmpty() || compareListings2(res, results1.get(results1.size() - 1)) < 0) {
					if (results1.size() == 100) {
						results1.remove(results1.size() - 1);
					}
					Lis[] l = new Lis[n + 1];
					System.arraycopy(res, 0, l, 0, n + 1);
					results1.add(l);
					Collections.sort(results1, (Lis[] l1, Lis[] l2) -> compareListings2(l1, l2)); // use strictly
																									// compareListings2
																									// instead of 1...
				}
			} else {
				if (results2.isEmpty() || compareListings2(res, results2.get(results2.size() - 1)) < 0) {
					if (results2.size() == 100) {
						results2.remove(results2.size() - 1);
					}
					Lis[] l = new Lis[n + 1];
					System.arraycopy(res, 0, l, 0, n + 1);
					results2.add(l);
					Collections.sort(results2, (Lis[] l1, Lis[] l2) -> compareListings2(l1, l2));
				}
			}
			return;
		}
		if (index == 0) {
			for (Sys sys : sl) {
				if (!used.contains(sys.id)) {
					used.add(sys.id);
					for (Sta sta : sys.stations) {
						for (Lis lis : sta.listings) {
							if (lis.buy_price > 0 && lis.supply >= minSupDem) {
								res[index] = lis;
								if (sta.isPlanetary)
									getAlgo2(sl, results1, results2, res, true, index + 1, n, used);
								else
									getAlgo2(sl, results1, results2, res, planets, index + 1, n, used);
							}
						}
					}
					used.remove(sys.id);
				}
			}
		} else {
			Lis lis1 = res[index - 1];
			Sta sta1 = stations.get(lis1.station_id);
			Sys sys1 = systems.get(sta1.system_id);
			for (Sys sys : sl) {
				if (!used.contains(sys.id)) {
					used.add(sys.id);
					for (Sta sta : sys.stations) {
						Lis lis2 = sta.listingsMap.get(lis1.com_id);
						if (lis2 != null && lis1.buy_price > 0 && lis1.buy_price < lis2.sell_price
								&& lis1.supply >= minSupDem && lis2.demand >= minSupDem) {
							for (Lis lis : sta.listings) {
								if (lis.buy_price > 0 && lis.supply >= minSupDem) {
									res[index] = lis2;
									res[index + 1] = lis;
									if (sta.isPlanetary)
										getAlgo2(sl, results1, results2, res, true, index + 2, n, used);
									else
										getAlgo2(sl, results1, results2, res, planets, index + 2, n, used);
								}
							}
						}
					}
					used.remove(sys.id);
				}
			}
		}
	}

	//////////

	int compareListings2(Lis[] a, Lis[] b) {
		int sum1 = 0;
		for (int i = 0; i < a.length - 1; i += 2) { // length-1 ...
			sum1 += a[i + 1].sell_price - a[i].buy_price;
		}
		int sum2 = 0;
		for (int i = 0; i < b.length - 1; i += 2) {
			sum2 += b[i + 1].sell_price - b[i].buy_price;
		}
		return Integer.compare(sum1, sum2);
	}

 

[kneitzel]

Gibts das ganz auch in nicht-obfuskiert?

Das sollte wohl mehr ein Hinweis in Richtung Clean Code sein. Ich will da keine Punkte aufwärmen und Dich erst Recht nicht überzeugen ... Aber evtl. nimmst Du so ein Erlebnis als Anlass, da den einen oder anderen Punkt noch einmal zu überdenken.

 

[Xyz1]

Wer sei jetzt angesprochen?
Ich kann mit mrbrowns Kommentar nicht viel anfangen.

 

[mrBrown]

Ich kann mit mrbrowns Kommentar nicht viel anfangen.

Obfuskation [...] beschreibt die absichtliche Veränderung von Programmcode, so dass der Quelltext für Menschen schwer verständlich [...] wird.

Dein Code ist der am schlechtesten verstehbare Code, den ich hier seit langem bei einer Frage gesehen habe. Das überrascht mich garnicht, dass da Fehler drin sind, und dass dir niemand helfen kann.

 

[Xyz1]

Mensch, ich habe es doch schon herausgefunden...
Sogar, wenn ich die Variablen abc... genannt hätte, hätte das mit der Fehlerfindung NICHTS zu tun gehabt... sprich, auch dann hätte man sehen können, dass etwas mit den Indices nicht stimmt.

 

[Meniskusschaden]

sprich, auch dann hätte man sehen können, dass etwas mit den Indices nicht stimmt.

Ach wie oft hätte man sich schon freuen können, wie schön man es hätte, wenn man das, was man hätte sehen können, aber nicht gesehen hatte, weil man es nicht so deutlich geschrieben hatte, wie man es hätte schreiben können, so geschrieben hätte, dass man es nicht nur hätte sehen können, sondern gesehen hätte.
Hat man aber nicht.

 

[kneitzel]

Ja, selbstverständlich, die Namensgebung und so wird einfach überschätzt. Daher ist meine Empfehlung auch, sich gar nicht mit der Namenswahl auseinander zu setzen sondern statt dessen nur einen UUID Generator zu verwenden und da dann einfach einen _ davor zu setzen. Egal ob Klassen, Methoden oder Variablen ...

Das hat dann auch die gute Konsequenz, dass man sich mit diesen albernden Paketen nicht herumschlagen muss - alles kann in default package bleiben .....

*Sarkasmus aus*

Clean Code musst Du nicht ernst nehmen. Du kannst Deinen Code so schreiben, wie Du willst. Ich finde so harsche Abwertungen wie von mrBrown auch nicht wirklich zielführend, weshalb ich da auch nur etwas in diese Richtung Clean Code pushen wollte.... Wenn Du List und Zeit hast, dann kannst Du Dir sowas wie clean-code-developer.de ansehen. Aber Du kannst das auch gerne sein lassen. MIR ist das komplett egal.

Aber diese Konversation ist nicht wirklich zielführend, daher klinke ich mich hier auch gleich wieder aus.

 

[Xyz1]

@Meniskusschaden und @kneitzel Es war ein Versuch gewesen, ob jemand den Indices-Fehler gesehen hätte, noch bevor ich ihn sah. Natürlich gibt es immer bessere Variablennamen. Jetzt habe ich auch die harsche Kritik von mrbrown verstanden und denke, man hätte sie vielleicht auch etwas freundlicher schreiben können...
Den ersten Teil (if (index > n) {) könnte man in eine Methode auslagern. Den längeren zweiten Teil würd ich aber so lassen wie er ist.
Zu den Variablennamen habe ich schon etwas geschrieben. Dem einem reicht n, der andere möchte numbersOfArrayElementsToCreateAndAdd haben. Was sei jetzt besser?

 

[kneitzel]

Und l, l1, l2 in einem Kontext zu haben ist für Dich nicht unübersichtlich? Und was ist Lis? Und was ist Algo, das du mit deiner Funktion bekommen willst? Und es ist ein get, also könnte man meinen, dass es ein Getter ist und dann rechnet er ggf. länger wo es nicht erwartet wird?

Und nun schreib Unit Tests mit 100% Code coverage. Mag bei Dir gehen, aber du kannst dir vorstellen, dass es schwer verständlich sein kann: was muss man für welchen Fall außen übergeben damit ein bestimmter of Abzweig innen erreicht wird?

Das aber wieder nur als reine Anregung bzw Erläuterung meiner vorherigen Anregung.