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

GSoC 2023: Yige Huang Week 3 #303

Merged
merged 12 commits into from Jun 18, 2023
Merged

Conversation

squarege
Copy link

@squarege squarege commented Jun 15, 2023

Changes proposed in this pull request:

  • Reverting the modification of docqueries and pgtap.
  • Creating new files for new function.
  • Adding new signatures.
  • Simplifying header file.
  • Fixing lincenses.

@pgRouting/admins

@squarege squarege added the yige-2023 Yige's GSoC 2023 project label Jun 15, 2023
@squarege squarege merged commit 4ef3f5d into pgRouting:yige-2023 Jun 18, 2023
4 of 10 checks passed


char
estimate_drivingSide_dd(char driving_side, bool directed, char** err_msg){
Copy link
Author

Choose a reason for hiding this comment

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

Here is the function I wrote to check the driving side. I am inexperienced in error checking, can you please give me some advice? @krashish8

Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest that if you are raising some error, why not raise it early, i.e. in the SQL code itself? You can look at PLPGSQL documentation on how to do so - a very small example might be:
https://github.com/pgRouting/pgrouting/blob/6c47f688c7ec219b5251b0020db4bc439ed2421a/sql/breadthFirstSearch/breadthFirstSearch.sql#L54-L57

Once the code of execution goes to C/C++, you can just set the default values of driving distance, assuming that the error checking is done by the SQL code. Inside this estimate_drivingSide_dd function, you can also try to use C++ functions if possible, such as std::tolower, std::string.find(), etc.

I added a comment in that issue - #300 (comment)
Have we finalized whether we are changing the default value of driving side to 'r', or will it be the same as before ('b'), or do we still need to check on that?

Copy link
Author

Choose a reason for hiding this comment

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

Last meeting, vicky says that I can make the check on SQL level or C/C++ level. But since I don't know how to check on the SQL level, I chose the latter. Thank you for your guidance and I will follow the PLPGSQL documentation and also the given example, trying to raise error on SQL level.

Actually, there will not any default value for driving side because I'm creating a new function that the driving side is compulsory rather than optional. User must give a proper value for driving side or the function will not work. Since there are some restrictions, like "Both" is only valid on undirected graphs (If b is given meanwhile graph is directed, I consider it should raise error because you cannot drive on both side in a directed graph), I think the check is needed.

Again, thanks for your guidance!

@squarege squarege deleted the yige-week-3 branch July 26, 2023 12:20
@squarege squarege changed the title GSoC 2023: Yige Huang week 3 GSoC 2023: Yige Huang Week 3 Aug 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
yige-2023 Yige's GSoC 2023 project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants