-
Notifications
You must be signed in to change notification settings - Fork 1
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
139 backend remove redundant calculations and database queries #147
139 backend remove redundant calculations and database queries #147
Conversation
@@ -91,6 +91,7 @@ export default class LogController { | |||
* @param trackerId - Tracker to filter for (Optional) | |||
* @returns Log[] - List of all logs | |||
*/ | |||
// TODO |
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.
Can you maybe elaborate a bit on this TODO with a comment after it? It might be important for Darlin as well and she might not directly know what you mean by that
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.
Just an artefact of testing things, because I mark temporary changes like that and (normally) search and remove them before comitting. So no TODO for @Niatsuna (will be fixed in next commit).
public static async getVehiclePosition(vehicle: Vehicle): Promise<GeoJSON.Feature<GeoJSON.Point> | null> { | ||
public static async getVehiclePosition( | ||
vehicle: Vehicle, | ||
vehicleHeading: 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.
Those new parameters are not mentioned in the JSDoc yet. Can you add them?
@@ -98,7 +102,12 @@ export default class VehicleService { | |||
// now it is unlikely, that weights can be added to the app logs, but we could at least try it | |||
logger.warn(`Could not add weights to tracker logs for vehicle with id ${vehicle.uid}.`) | |||
} else { | |||
const tempWeightedTrackKm = await this.weightedLogsToWeightedTrackKm(weightedTrackerLogs, track) | |||
const tempWeightedTrackKm = await this.weightedLogsToWeightedTrackKm( |
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.
The if-case above seems to be impossible, as addWeightToLogs always returns a list and never null
@@ -119,7 +128,12 @@ export default class VehicleService { | |||
logger.warn(`Could not add weights to app logs for vehicle with id ${vehicle.uid}.`) |
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.
This statement will never be reached as addWeightToLogs never returns null
@@ -156,6 +170,8 @@ export default class VehicleService { | |||
*/ | |||
private static async weightedLogsToWeightedTrackKm( | |||
weightedLogs: [Log, number][], | |||
vehicleSpeed: 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.
Those new parameters are not mentioned in the JSDoc yet
public static async getVehicleTrackHeading( | ||
vehicle: Vehicle, | ||
trackKm: number, | ||
vehicleHeading: 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.
The new parameters are not mentioned in the JSDoc yet
ok I'm just going to bother you once more ;) the JSDoc in getvehicleTrackHeading doesn't contain the optional |
Improving redundancy resulting in a better performance (see #139).