added merge() funtion and test cases - #43
Conversation
|
|
||
| return head | ||
|
|
||
| def merge_linked_list(self, list2): |
There was a problem hiding this comment.
Good Effort and pull request !
rename the function to sorted_merge.
Also in the parameter you are not taking two linked list head.
for example: user should be able to pass 2 linkedlist in the parameter to get the head of new sorted linkedlist.
so there is no use of self it should be declared explicitly because we want this as a linkedlist method.
@devkapilbansal you can also suggest something.
| """ | ||
| :param: head - head of first linked list | ||
| :param: list2 - head of second linked list | ||
| return - return a new merged linked list |
There was a problem hiding this comment.
it should be return a new sorted merged list.
| array = self.to_list() | ||
|
|
||
| linked_list = LinkedList() | ||
| for value in array: |
There was a problem hiding this comment.
you can also reduce these steps but it's fine.
There was a problem hiding this comment.
Do I need to use a diff approach for merge() ?
|
@hemabhagnani this pull request looks good to me and will be ready to merge after mentioned changes. |
|
@codesankalp I'll do the suggested changes |
|
@hemabhagnani are you working on this? |
|
Actually, I wanted to ask whether I have to change the approach of the |
|
|
||
| def __repr__(self): | ||
| return "LinkedList({})".format((self.to_list())) | ||
|
|
There was a problem hiding this comment.
define the sorted_merge function explicitly i.e. not in the class.
and also don't remove this function and use the self instead of list1.
so this is the final instructions:-
- in the current sorted_merge function use self instead of list1.
- define a new function sorted merge outside the class which takes two args list1 and list2 and returns merged linked list.
|
@codesankalp @17sushmita review it |
devkapilbansal
added
Status: Review Required
and removed
Status: Changes Requested
labels
3 participants
fixes #35