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

Refactor: improve query parsing logic and update JSDoc #6136

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ayoub-Mabrouk
Copy link

  • Updated the query parsing function to utilize modern JavaScript features.
  • Changed the JSDoc return type from {String} to {Object} to accurately describe the return value.
  • Enhanced readability and consistency in the implementation.

- Updated the query parsing function to utilize modern JavaScript features.
- Changed the JSDoc return type from {String} to {Object} to accurately describe the return value.
- Enhanced readability and consistency in the implementation.
Copy link

@Abdul-Raffay-0 Abdul-Raffay-0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change from var to const can have dramatic effects, if you dont know the whole subject, please refrain from mass commits in the name of clean code. There is a reason for everything

@Ayoub-Mabrouk
Copy link
Author

The change from var to const can have dramatic effects, if you dont know the whole subject, please refrain from mass commits in the name of clean code. There is a reason for everything

Hello @Abdul-Raffay-0 Thanks for your comment! I wanted to clarify the recent change from var to const for queryparse in the defineGetter function. This shift is actually a positive move for our code. By using const, we prevent accidental reassignment, which helps keep queryparse stable and predictable within its specific scope. This is especially useful in larger codebases where variables can easily get mixed up.

The defineGetter function itself is used to create convenient properties on the req object—like req.query, req.protocol, and req.ip. Each property has a getter function for easy access, and using const for queryparse keeps it isolated from other parts of the code, ensuring no unintended side effects.

I also appreciate your input about mass commits. It's a good reminder that we should double-check our understanding before making strong statements. It's always better to confirm that we’re on the same page first.

If you’d like to chat more about this or have any other questions, feel free to reach out. Have a wonderful day!

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.

2 participants