Skip to content

added addressing support to linked list - #66

Open
ravi5175 wants to merge 4 commits into
codesankalp:masterfrom
ravi5175:master
Open

added addressing support to linked list#66
ravi5175 wants to merge 4 commits into
codesankalp:masterfrom
ravi5175:master

Conversation

@ravi5175

@ravi5175 ravi5175 commented Oct 2, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Description

  • added support for node addressing in to_list() function
  • improved test case for enhanced usecases of Nodes.

Related Issues

Fixes #65

Type of Change:

  • Code
  • Quality Assurance
  • Documentation

Code/Quality Assurance Only

  • This change requires a documentation update (software upgrade on readme file)

How Has This Been Tested?

proper testing has been done in the test/linkedlist.ipynb file

Checklist:

  • I read the CONTRIBUTING document before making this pull request
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
  • My change requires change to documentation
  • I have updated documentation accordingly
  • I have updated the requirements file
  • My changes generate no new warnings
  • My PR currently breaks something (fix or feature that would cause existing functionality to not work as expected)
  • I have added tests to cover my changes
  • All new and existing tests passed

Screenshots (if appropriate):

image

codesankalp
codesankalp reviewed Oct 3, 2020
View reviewed changes
Comment thread dsalgo/linked_list.py Outdated
node = self.head
while node:
out_list.append(node.value)
out_list.append([node.value,hex(id(node))])

@codesankalp codesankalp Oct 3, 2020

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

add space after comma (can use flake 8)

@ravi5175 ravi5175 Oct 3, 2020

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

well that's some unneccessary change and i use pylinter

@codesankalp

codesankalp commented Oct 3, 2020

Copy link
Copy Markdown
Owner

@ravi5175 @devkapilbansal will it be fine if we add address when converting the linkedlist to python list. because if a user needs conversion of linked list to python list then why we are providing the addresses also.
So I think that we should add a new method for this like to_list_with_address
Also in the linkedlist tests to list method is very important part for testing.

@ravi5175

ravi5175 commented Oct 3, 2020

Copy link
Copy Markdown
Collaborator Author

@ravi5175 @devkapilbansal will it be fine if we add address when converting the linkedlist to python list. because if a user needs conversion of linked list to python list then why we are providing the addresses also.
So I think that we should add a new method for this like to_list_with_address
Also in the linkedlist tests to list method is very important part for testing.

@codesankalp it is still a list with a advanced usecase where a user can manually access the node located at some address, it can also be used used as a list with node values.

@ravi5175
argumental addressing support
7b760d2 
Signed-off-by: Ravi Pawar <ravipawar5175@gmail.com>
@devkapilbansal

devkapilbansal commented Oct 3, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator

@ravi5175 change maintainers file to AUTHOR

Add Contributors and Authors there

There will be three section:
Author
Contributor
Maintainer

@devkapilbansal devkapilbansal added the hacktoberfest-accepted Accept for hacktoberfest - will merge later label Oct 3, 2020
@devkapilbansal

devkapilbansal commented Oct 7, 2020

Copy link
Copy Markdown
Collaborator

@ravi5175 resolve conflicts and rebase your current branch with our master branch

@Aryamanz29

Aryamanz29 commented Oct 2, 2021

Copy link
Copy Markdown
Collaborator

@ravi5175 Please do req changes, So that we move ahead with this PR
Thanks

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

Reviewers

@codesankalp codesankalp codesankalp left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

hacktoberfest-accepted Accept for hacktoberfest - will merge later Status: Changes Requested

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Required Node addressing in to_list() function in linked list

4 participants

@ravi5175 @codesankalp @devkapilbansal @Aryamanz29