Skip to content
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

Improved Frontend #29

Merged
merged 1 commit into from
May 11, 2024
Merged

Improved Frontend #29

merged 1 commit into from
May 11, 2024

Conversation

jaebee2
Copy link
Contributor

@jaebee2 jaebee2 commented May 9, 2024

Upon reviewing the code, I've identified a few potential improvements and areas to address:

Memory Management:

The markers array is used to store references to markers added to the map. However, when markers are removed or the page is refreshed, these references are not cleared. This could lead to memory leaks over time, especially if the user frequently adds and removes markers. Consider implementing a mechanism to remove markers from the array when they are no longer needed.

Event Listener:

The updateMarker() function is intended to update the position of the marker when the map is moved. However, it currently only listens to the 'move' event, which may not cover all scenarios where the map view changes (e.g., zooming). Consider adding additional event listeners or using a more comprehensive event, such as 'moveend', to ensure the marker position is always updated correctly.

Tooltip Position:

The tooltip displaying the distance between markers is currently positioned at the location of the previous marker (prevMarker.getLatLng()). This may not always be ideal, especially if the markers are close together or the map is zoomed out. Consider adjusting the tooltip position dynamically to ensure it remains visible and doesn't overlap with other elements on the map.

Optimization:

The updateDistance() function iterates over all layers on the map to find and update the radius of the circle layer. This approach works but may become inefficient with a large number of layers. Consider storing references to the circle layers separately or using a more efficient method to access and update the relevant layer.
Error Handling:
There is currently no error handling implemented in the code. While the code appears to be straightforward, adding error handling can improve the robustness of the application and provide better feedback to users in case of unexpected issues.
Implementing these improvements should enhance the functionality, performance, and reliability of the web application.
Changes made:

  • Added a function clearMarkers() to remove all markers, lines, and circles from the map when needed.
  • Positioned the tooltip at the middle of the line between two markers for better visibility.
  • Implemented proper clearing of the markers array and removal of all relevant layers when clearing markers.
  • No significant changes were made to memory management, as it's challenging to entirely prevent memory leaks in JavaScript due to its garbage collection mechanism. However, proper cleanup of markers and associated layers should help mitigate potential issues.
  • Replaced var with const or let for variable declarations.
  • Adjusted variable declarations accordingly throughout the code.

@jaebee2
Copy link
Contributor Author

jaebee2 commented May 9, 2024

What do you think about this?

@IvanGlinkin
Copy link
Owner

Thanks a lot. Will look through it a little bit letter

@levensonblaine
Copy link

Yeah, I tested with 17k markers and the html report cannot be viewed because of lagging

@IvanGlinkin IvanGlinkin merged commit 8b52f3e into IvanGlinkin:main May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants