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.

Sign up for GitHub

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

Jump to bottom

Bresenham line programs in java & python #4070

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Bresenham line programs in java & python #4070

wants to merge 4 commits into from

Conversation

robotjellyzone
Copy link
Contributor

@robotjellyzone robotjellyzone commented May 11, 2019 •
edited

Fixes issue:#4024

Changes:

*This algo generates a line by using a graphics library . Codes are implemented in java & python with outputs provided for each code.

contactvaibhavi
contactvaibhavi suggested changes May 12, 2019
View reviewed changes
Copy link
Member

@contactvaibhavi contactvaibhavi left a comment

Choose a reason for hiding this comment

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

Pls do the changes asked in your earlier Pull request too.

code/computational_geometry/src/bresenham_line/bresenhamLine.java Outdated
x++;
sum -= Dy;
if (sum < 0) {
y = y + prirastokDy;
Copy link
Member

Choose a reason for hiding this comment

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

change the name of this variable - prirastokDy.

code/computational_geometry/src/bresenham_line/Bresenham_Line_Algorithm.py Outdated
@@ -0,0 +1,63 @@
# Import graphics library
from graphics import *
Copy link
Member

Choose a reason for hiding this comment

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

Import only relevant functions, not the entire graphics library.

contactvaibhavi
contactvaibhavi suggested changes May 19, 2019
View reviewed changes
Copy link
Member

@contactvaibhavi contactvaibhavi left a comment

Choose a reason for hiding this comment

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

Good work overall

code/data_structures/src/stack/infix_to_prefix/Infix_To_Prefix.cpp
@@ -0,0 +1,128 @@
// Including Library
#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.

Add relevant header files instead of <bits/stdc++.h>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, @vaibhavi24 this is not the relevant file!!
You are by mistake reviewing another file!! Please see only the relevant files of Bresenhams

This one is of infix to prefix!!!

Copy link
Member

Choose a reason for hiding this comment

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

@robotjellyzone Could you remove this file from this PR if it is unrelated? Looks like your branches may have combined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@arnavb I have shared this issue with @AdiChat . As there some issues with my local repo so I am unable to do so

@contactvaibhavi contactvaibhavi mentioned this pull request May 19, 2019
Closed
@robotjellyzone
Copy link
Contributor Author

Hi, @vaibhavi24 please review the above two files of java and python!! With commit message "java file of Bresenham" and "python file of Bresenham".

The one you reviewed was not relevant one as that was of c++ infix to prefix!!

@robotjellyzone
Copy link
Contributor Author

@arnavb please review it!!

contactvaibhavi
contactvaibhavi approved these changes May 26, 2019
View reviewed changes
Copy link
Member

@contactvaibhavi contactvaibhavi left a comment

Choose a reason for hiding this comment

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

Changes done

@robotjellyzone robotjellyzone mentioned this pull request May 30, 2019
Merged
@robotjellyzone
Copy link
Contributor Author

I have opened a new pull request to remove the unrelated files !!

@adi-g15
Copy link

adi-g15 commented Aug 19, 2022

This should be closed as #4091 has been merged.
Actually, this was suggested by codetriage to help triage open source issues :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arnavb arnavb arnavb left review comments

@contactvaibhavi contactvaibhavi contactvaibhavi approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@robotjellyzone @adi-g15 @arnavb @contactvaibhavi