Skip to content

feat: added a new algorithm to find whether two line segment intersec… #796

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

Merged
merged 12 commits into from
May 30, 2020
Merged

Conversation

meSajied
Copy link
Contributor

@meSajied meSajied commented May 26, 2020

…t or not

Description of Change

Checklist

  • Added description of change
  • Added file name matches File name guidelines
  • Added tests and example, test must pass
  • Relevant documentation/comments is changed or added
  • PR title follows semantic commit guidelines
  • Search previous suggestions before making a new one, as yours may be a duplicate.
  • Sort by alphabetical order
  • I acknowledge that all my contributions will be made under the project's license.

Notes:

@@ -0,0 +1,63 @@
#include <bits/stdc++.h>
Copy link
Member

Choose a reason for hiding this comment

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

don't use <bits/stdc++.h>

#include <bits/stdc++.h>
using namespace std;

class Point{
Copy link
Member

Choose a reason for hiding this comment

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

you can use a struct if all the members are public

Copy link
Member

@ayaankhan98 ayaankhan98 left a comment

Choose a reason for hiding this comment

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

cpplint_modified_files is failling

geometry/line_segment_intersection.cpp:2:  Do not use namespace using-directives.  Use using-declarations instead.  [build/namespaces] [5]
geometry/line_segment_intersection.cpp:5:  public: should be indented +1 space inside class Point  [whitespace/indent] [3]
geometry/line_segment_intersection.cpp:10:  public: should be indented +1 space inside class SegmentIntersection  [whitespace/indent] [3]
geometry/line_segment_intersection.cpp:11:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
geometry/line_segment_intersection.cpp:11:  Missing space before {  [whitespace/braces] [5]
geometry/line_segment_intersection.cpp:13:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
geometry/line_segment_intersection.cpp:17:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
geometry/line_segment_intersection.cpp:17:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
geometry/line_segment_intersection.cpp:18:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
geometry/line_segment_intersection.cpp:20:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
geometry/line_segment_intersection.cpp:21:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
geometry/line_segment_intersection.cpp:23:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
geometry/line_segment_intersection.cpp:24:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
geometry/line_segment_intersection.cpp:26:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
geometry/line_segment_intersection.cpp:27:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
geometry/line_segment_intersection.cpp:29:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
geometry/line_segment_intersection.cpp:30:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
geometry/line_segment_intersection.cpp:32:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
geometry/line_segment_intersection.cpp:37:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
geometry/line_segment_intersection.cpp:37:  Missing space before {  [whitespace/braces] [5]
geometry/line_segment_intersection.cpp:38:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
geometry/line_segment_intersection.cpp:38:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
geometry/line_segment_intersection.cpp:39:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
geometry/line_segment_intersection.cpp:42:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
geometry/line_segment_intersection.cpp:43:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]

@ayaankhan98 ayaankhan98 added the automated tests are failing Do not merge until tests pass label May 26, 2020
Copy link
Collaborator

@kvedala kvedala left a comment

Choose a reason for hiding this comment

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

  1. repository is meant for educational purposes and hence, the codes submitted must be educative i.e., others should be able to read and learn from them. This would require the code to be documented as much as possible.
  2. please ensure that you have performed the sanity checks mentioned in the pull request template. one of them is to READ the CONTRIBUTIONS.md file. Therein is a mention that the files should conform to cpplint and all the help for the same is also mentioned.
  3. Please review the protocols and make the necessary corrections.

@@ -0,0 +1,63 @@
#include <bits/stdc++.h>
using namespace std;
Copy link
Collaborator

Choose a reason for hiding this comment

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

fails cpplint. must be removed.

int x, y;
};

class SegmentIntersection{
Copy link
Collaborator

Choose a reason for hiding this comment

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

document your code.

@deadshotsb deadshotsb added the awaiting modification Do not merge until modifications are made label May 27, 2020
@meSajied meSajied requested a review from ayaankhan98 May 27, 2020 13:28
Copy link
Member

@ayaankhan98 ayaankhan98 left a comment

Choose a reason for hiding this comment

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

this all looks good but do one more thing add a breif documentation for each function, it will help the reader to understand the code in better way

@kvedala
Copy link
Collaborator

kvedala commented May 28, 2020

Please see the file search/binary_search.h for example of documentation.

@kvedala
Copy link
Collaborator

kvedala commented May 29, 2020

This looks much better, thank you.
Now, can you structure it in the doxygen-format as shown in search/binary_search.h and described here. This will help the system to automatically build documentation like this: https://kvedala.github.io/C-Plus-Plus without manual intervention.

@kvedala
Copy link
Collaborator

kvedala commented May 29, 2020

Excellent. Now, the beginning of the file should contain a comment block like so:

/**
 * \file
 * \brief -- Brief one line comment here
 * \description -- optional multiline large description here
 */

@kvedala
Copy link
Collaborator

kvedala commented May 29, 2020

also, the comment blocks should go just above the function declaration

int x, y;
};

struct SegmentIntersection {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this struct is undocumented

@@ -0,0 +1,88 @@
#include <iostream>

struct Point {
Copy link
Collaborator

Choose a reason for hiding this comment

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

struct is undocumented.
you can add a brief comment like so: others/smallest_circle

struct SegmentIntersection {
inline bool intersect(Point first_point, Point second_point,
Point third_point, Point forth_point) {
/**
Copy link
Collaborator

Choose a reason for hiding this comment

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

comment blocks should go above the function name i.e. for example, the comment block between lines 10-14 should be just above line 8.


inline int direction(Point first_point, Point second_point,
Point third_point) {
/**
Copy link
Collaborator

Choose a reason for hiding this comment

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

same - comment block from lines 46-54 should be at line 44.

}
};

int main() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

document the main function. a simple one-liner is good

Copy link
Collaborator

@kvedala kvedala left a comment

Choose a reason for hiding this comment

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

there should be a one line gap between end of a previous code and start of a new comment/code. But the cpplint does not seem to give any errors regarding that. So, I approve as well.

Good work, @meSajied Thank you

@meSajied
Copy link
Contributor Author

meSajied commented May 30, 2020

Can you merge this pull request now @kvedala??

@ayaankhan98 ayaankhan98 removed automated tests are failing Do not merge until tests pass awaiting modification Do not merge until modifications are made labels May 30, 2020
return false;
}
};
/*
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/*
/**

* or not.
*/
#include <iostream>
/*
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/*
/**

Copy link
Member

@ayaankhan98 ayaankhan98 left a comment

Choose a reason for hiding this comment

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

everything else looks good

@meSajied
Copy link
Contributor Author

meSajied commented May 30, 2020

I think everything is good now.... @ayaankhan98

Copy link
Member

@ayaankhan98 ayaankhan98 left a comment

Choose a reason for hiding this comment

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

yes looks fine

@ayaankhan98 ayaankhan98 merged commit 0eedbb3 into TheAlgorithms:master May 30, 2020
@kvedala
Copy link
Collaborator

kvedala commented May 31, 2020

Reason why we need a robust CI - build fails on windows due to missing #include <algorothm> for the functions std::max and std::min

@meSajied
Copy link
Contributor Author

meSajied commented Jun 1, 2020

But now pull request is merged... @kvedala

@kvedala
Copy link
Collaborator

kvedala commented Jun 1, 2020

you can recreate a pull request with corrections. If you make a pull request - here, it will compile and check your code automatically

but I have already fixed your code there. Thats how I found out. Otherwise, it is impossible for one man to compile and check each every one of 100+ programs

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.

4 participants