Skip to content

Adding graham.c #106

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 4 commits into from
May 14, 2018
Merged

Adding graham.c #106

merged 4 commits into from
May 14, 2018

Conversation

Gathros
Copy link
Contributor

@Gathros Gathros commented May 11, 2018

No description provided.

@Gathros Gathros added the Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) label May 11, 2018
};

int cmp_points(const void *a, const void *b) {
struct point point1 = *(struct point*)a;
Copy link
Contributor

@zsparal zsparal May 12, 2018

Choose a reason for hiding this comment

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

You shouldn't copy the points, it's completely unnecessary. Just cast to struct point* and compare them that way

}
}

double ccw(struct point a, struct point b, struct point c) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think ccw is the best name here. That would make sense if it returned a bool. I'd probably call it orientation or something like that.

@Gathros
Copy link
Contributor Author

Gathros commented May 12, 2018

@Gustorn I changed ccw to is_left_of and changed cmp_points.

};

int cmp_points(const void *a, const void *b) {
if (((struct point*)a)->y > ((struct point*)b)->y) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Having them as variables wasn't a bad idea, I just didn't like the copy:

struct point* pa = (struct point*) a;
struct point* pb = (struct point*) b;

if (pa->y > pb->y) {
// ...

Copy link
Member

@june128 june128 left a comment

Choose a reason for hiding this comment

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

Should be working?

@june128 june128 merged commit 81cb3be into algorithm-archivists:master May 14, 2018
@june128
Copy link
Member

june128 commented May 14, 2018

Since @Gustorn approved it, it should be fine :)

@Gathros Gathros deleted the CGrahamPR branch May 20, 2018 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants