-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Accounting #159
base: master
Are you sure you want to change the base?
Accounting #159
Conversation
test_data/default/tour.sql
Outdated
('2024-07-09 11:00:00', '2024-07-09 11:30:00', 1, 3.50, 13.50), | ||
('2024-06-07 20:30:00', '2024-06-07 21:30:00', 1, 3.50, 9), | ||
('2024-05-05 10:30:00', '2024-05-05 12:00:00', 1, 3.50, 14), | ||
('2024-01-01 22:30:00', '2024-01-01 23:00:00', 1, 3.50, 15.50), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Die Spalten "fare" und "fare_route" stellen Centbeträge dar -> ganzzahlig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok, ich hatte mich schon gewundert, warum in der Datenbank Integer für diese Beträge drin steht.
{ | ||
for (let row of data.tours) { | ||
if ( | ||
row.from.getMonth() == 0 || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das Jahr wird nicht geprüft
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das stimmt, das habe ich vergessen zu überprüfen.
if (fare == null || fare_route == null) { | ||
return 0; | ||
} | ||
return (fare_route - fare) * 0.97; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0.97 als Konstante mit aussagekräftigem Namen,
Preis kann negativ werden, ist das Absicht?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Da wir noch nicht wissen, wie genau die Kosten berechnet werden, habe ich das zum Testen so gelöst.
Siehe auch die offenen Fragen im Issue #157.
let totalPages = $state(firstarray); | ||
let currentPageRows = $state(firstPage); | ||
|
||
const paginate = (tours: TourDetails[]) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funktion wird mehrfach definiert -> auslagern?
}; | ||
|
||
const sort = (des: boolean[], idx: number) => { | ||
if (des[idx]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Die Funktion hat einige Stellen an denen sich Sachen wiederholen, kann man das so in der Art verkürzen und evtl auch einfacher machen:
switch (idx) {
case 0:
currentRows.sort(compareDate);break;
case 1:
currentRows.sort(compareFareRoute);break;
case 2:
currentRows.sort(compareTotalPrice);break;
case 3:
currentRows.sort(compareCost);break;
default: return;
}
descending[idx] = !des[idx];
if(!des[idx]) {
currentRows.reverse();
}
paginate(currentRows);
setPage(0);
return customers.size; | ||
}; | ||
|
||
// fare = ÖV-Preis --- fare_route = Taxameterpreis |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
evtl dann direkt taximeterFare als Variablenname?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So heißen die Variablen in der db, das war nur für mich, damit ich die nicht durcheinander bringe.
setCompanys(currentRows); | ||
}); | ||
|
||
const setPage = (p: number) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funktion wird mehrfach definiert -> auslagern?
}; | ||
|
||
const compareTotalPrice = (a: TourDetails, b: TourDetails) => { | ||
let aTotal = getTotalPrice(a.fare, a.fare_route); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Warum nicht const statt let?
return 0; | ||
} | ||
let diff = Math.max(0, fare_route - fare); | ||
if (diff > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Zeile 40 liefert auch für diff=0 das korrekte Ergebnis, oder täusche ich mich?
Dann würde ich vorschlagen die Bedingung hier fallen zu lassen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jaein, es soll ja abgefangen werden, dass es negativ wird. Bei fare_route - fare = 0 ist es eigentlich egal, weil so oder so 0 rauskommt.
Ich habe den Code trotzdem mal für bessere Lesbarkeit geändert.
Siehe Issue #157 für eine Übersicht der abgeschlossenen und noch offenen Aufgaben.