Skip list #941

enqidu · 2020-07-07T09:37:33Z

Description of Change

Checklist

Added description of change
Added file name matches File name guidelines
Added tests and example, test must pass
PR title follows semantic commit guidelines
I acknowledge that all my contributions will be made under the project's license.

Notes:
Skip List is very useful data structure for searching in sparse and big data.

lgtm-com · 2020-07-07T10:15:21Z

This pull request introduces 1 alert when merging f13964c into 5baf1ad - view on LGTM.com

new alerts:

1 for Resource not released in destructor

enqidu · 2020-07-07T10:37:54Z

Can someone tell me why is it failing?

lgtm-com · 2020-07-07T10:38:31Z

This pull request introduces 1 alert when merging 2fb95a2 into 5baf1ad - view on LGTM.com

new alerts:

1 for Resource not released in destructor

kvedala · 2020-07-07T10:45:08Z

Can someone tell me why is it failing?

Please click on details and you'll find the descriptions for exactly why the tests are failing.

enqidu · 2020-07-07T10:48:59Z

Can someone tell me why is it failing?

Please click on details and you'll find the descriptions for exactly why the tests are failing.

thanks

lgtm-com · 2020-07-07T11:04:25Z

This pull request introduces 1 alert when merging 910180f into 5baf1ad - view on LGTM.com

new alerts:

1 for Resource not released in destructor

enqidu · 2020-07-07T11:20:19Z

@kvedala I have this error "Consider using rand_r(...) instead of rand(...) for improved thread safety", but when I switch to rand_r windows tests fail (even though I include #include library). Could you tell me how can I resolve this?

lgtm-com · 2020-07-07T11:26:02Z

This pull request introduces 1 alert when merging 8e99383 into 5baf1ad - view on LGTM.com

new alerts:

1 for Resource not released in destructor

kvedala · 2020-07-07T12:16:58Z

@kvedala I have this error "Consider using rand_r(...) instead of rand(...) for improved thread safety", but when I switch to rand_r windows tests fail (even though I include #include library). Could you tell me how can I resolve this?

Much of the code uses C header files and C functions. If you want to continue using them, the code will be more appropriate for the https://github.com/TheAlgorithms/C repo.
For C++, the correct way to invoke randr function is: std::randr. Moreover, before invoking the rand function, the generator needs to be initialized: std::srand(std::time(nullptr)).
Following C++ standards should help you get closer to overcoming the errors and alerts reported by the auto-checks.

For more info, please refer to existing code in the repo for reference. There are many programs that use the above functions and should help you learn how to best use them.

Let me also give you a heads up about the documentation standards - we use Doxygen to autogenerate the website - https://thealgorithms.github.io/C-Plus-Plus

kvedala · 2020-07-07T12:18:58Z

Please also enable GitHub Actions in your fork so that the auto tools can help auto-format the code for you

enqidu · 2020-07-07T13:02:11Z

@kvedala I have this error "Consider using rand_r(...) instead of rand(...) for improved thread safety", but when I switch to rand_r windows tests fail (even though I include #include library). Could you tell me how can I resolve this?

Much of the code uses C header files and C functions. If you want to continue using them, the code will be more appropriate for the https://github.com/TheAlgorithms/C repo.

For C++, the correct way to invoke randr function is: std::randr. Moreover, before invoking the rand function, the generator needs to be initialized: std::srand(std::time(nullptr)).

Following C++ standards should help you get closer to overcoming the errors and alerts reported by the auto-checks.

For more info, please refer to existing code in the repo for reference. There are many programs that use the above functions and should help you learn how to best use them.

Let me also give you a heads up about the documentation standards - we use Doxygen to autogenerate the website - https://thealgorithms.github.io/C-Plus-Plus

Thank you, I've polished code following C++ standards

lgtm-com · 2020-07-07T13:04:10Z

This pull request introduces 1 alert when merging 5068ddc into 5baf1ad - view on LGTM.com

new alerts:

1 for Resource not released in destructor

kvedala · 2020-07-07T13:10:04Z

This pull request introduces 1 alert when merging 5068ddc into 5baf1ad - view on LGTM.com

new alerts:

1 for Resource not released in destructor

☝️

kvedala

Apart from the suggested changes and possible sources of errors, the code lacks proper memory management.
Execution leads to segmentation faults as shown in the screenshot. Please re-assess the code and try to address the concerns.

Thank you for your contribution. Looking to see the corrections and a working code.

data_structures/skip_list.cpp

Co-authored-by: Krishna Vedala <7001608+kvedala@users.noreply.github.com>

enqidu · 2020-07-11T14:02:18Z

Hello, @kvedala I merged commits as you told me.

kvedala · 2020-07-11T17:13:37Z

data_structures/skip_list.cpp

+ void* value;
+ // Forward Array points to the neighbour (right)
+ // nodes of the given one in all levels
+ vector<Node*> forward;


Class variables still not commented correctly. Please refer the following

Doxygen details

CONTRIBUTION.md

data_structures/skip_list.cpp

kvedala · 2020-07-11T17:14:59Z

Thank you. Much has been resolved. Just the class member variables are not documented per standard.

kvedala · 2020-07-11T22:42:23Z

Note:

The formatter is trying to apply some fixes. Please enable "GitHub Actions" on your fork
Better to use std::unique_ptr where possible. I have disabled this as an error but is a good CPP Standard Recommendation

…o skip_list

kvedala · 2020-07-12T16:37:35Z

Doc fixes look good 👍
Self tests are failing

enqidu · 2020-07-12T17:20:34Z

Doc fixes look good
Self tests are failing

Yeah, I have no idea why, I just changed comments

kvedala · 2020-07-12T18:01:24Z

Doc fixes look good
Self tests are failing

Yeah, I have no idea why, I just changed comments

We recently added stricter checks on the code submitted (see #947)

enqidu · 2020-07-12T18:36:23Z

Doc fixes look good
Self tests are failing

Yeah, I have no idea why, I just changed comments

We recently added stricter checks on the code submitted (see #947)

Sorry. I can't work on this any longer.

kvedala · 2020-07-12T20:22:48Z

Doc fixes look good
Self tests are failing

Yeah, I have no idea why, I just changed comments

We recently added stricter checks on the code submitted (see #947)

Sorry. I can't work on this any longer.

Thank you for your tries and contribution, I'll for it to my branch and fix it further by myself. Your commits will retain your history so that you get due credit for your work.

…ired

kvedala · 2020-07-12T21:16:48Z

Is this the expected output?

enqidu · 2020-07-13T09:14:37Z

Is this the expected output?

Yes. Numbers are randomized so every execution will give different output but list display shows us that it's working.

enqidu · 2020-07-13T09:16:54Z

Doc fixes look good
Self tests are failing

Yeah, I have no idea why, I just changed comments

We recently added stricter checks on the code submitted (see #947)

Sorry. I can't work on this any longer.

Thank you for your tries and contribution, I'll for it to my branch and fix it further by myself. Your commits will retain your history so that you get due credit for your work.

Okay. Good luck

data_structures/skip_list.cpp

ayaankhan98

LGTM

enqidu added 5 commits July 7, 2020 13:30

added skip_list.cpp

d11e8cb

added skip-list

ec04afc

skip_list

94338f5

skip_list

2c6b8c3

skip-list

f13964c

skip_list

2fb95a2

skip_list

420915f

skip_list

910180f

skip_list

8e99383

enqidu added 3 commits July 7, 2020 16:40

skip-list corrected

2fc0d5b

skip-list documentation-polish

ba65d55

skip-list documentation-polish

5068ddc

skip-list documentation-polish

dcdebae

kvedala requested changes Jul 7, 2020

View reviewed changes

enqidu added 3 commits July 7, 2020 18:16

skip list update

f7ab869

skip_list

fd8affb

skip_list

4cebe7c

enqidu and others added 3 commits July 10, 2020 21:27

Update data_structures/skip_list.cpp

b1b3b31

Co-authored-by: Krishna Vedala <7001608+kvedala@users.noreply.github.com>

Update data_structures/skip_list.cpp

5d225ab

Co-authored-by: Krishna Vedala <7001608+kvedala@users.noreply.github.com>

Update data_structures/skip_list.cpp

af49a02

Co-authored-by: Krishna Vedala <7001608+kvedala@users.noreply.github.com>

kvedala reviewed Jul 11, 2020

View reviewed changes

data_structures/skip_list.cpp Outdated Show resolved Hide resolved

enqidu added 2 commits July 12, 2020 19:24

update

a707d71

Merge branch 'skip_list' of https://github.com/enqidu/C-Plus-Plus int…

ddf148b

…o skip_list

kvedala added 2 commits July 12, 2020 17:00

tidied up code based on error reports by clang-tidy

f970ce3

since we are using smart pointers, an explicit destructor is not requ…

c15c962

…ired

fix randomlevel float conversion

47f5ad9

kvedala requested a review from ayaankhan98 July 13, 2020 12:06

kvedala reviewed Jul 13, 2020

View reviewed changes

data_structures/skip_list.cpp Outdated Show resolved Hide resolved

fix doc comment block

dc4a72c

kvedala reviewed Jul 13, 2020

View reviewed changes

data_structures/skip_list.cpp Show resolved Hide resolved

main function must return

eefe466

ayaankhan98 approved these changes Jul 13, 2020

View reviewed changes

kvedala approved these changes Jul 13, 2020

View reviewed changes

ayaankhan98 merged commit ef13806 into TheAlgorithms:master Jul 13, 2020

kvedala linked an issue Jul 13, 2020 that may be closed by this pull request

[FEATURE] Skip List #939

Closed

Uh oh!

Skip list #941

Skip list #941

Uh oh!

Conversation

enqidu commented Jul 7, 2020 • edited by gitpod-io bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Checklist

lgtm-com bot commented Jul 7, 2020

enqidu commented Jul 7, 2020

lgtm-com bot commented Jul 7, 2020

kvedala commented Jul 7, 2020

enqidu commented Jul 7, 2020

lgtm-com bot commented Jul 7, 2020

enqidu commented Jul 7, 2020

lgtm-com bot commented Jul 7, 2020

kvedala commented Jul 7, 2020

kvedala commented Jul 7, 2020

enqidu commented Jul 7, 2020

lgtm-com bot commented Jul 7, 2020

kvedala commented Jul 7, 2020

kvedala left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

enqidu commented Jul 11, 2020

kvedala Jul 11, 2020

Choose a reason for hiding this comment

Uh oh!

kvedala commented Jul 11, 2020

kvedala commented Jul 11, 2020

kvedala commented Jul 12, 2020

enqidu commented Jul 12, 2020

kvedala commented Jul 12, 2020

enqidu commented Jul 12, 2020

kvedala commented Jul 12, 2020

kvedala commented Jul 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

enqidu commented Jul 13, 2020

enqidu commented Jul 13, 2020

Uh oh!

Uh oh!

ayaankhan98 left a comment

Choose a reason for hiding this comment

Labels

4 participants

enqidu commented Jul 7, 2020 •

edited by gitpod-io bot

Loading

kvedala commented Jul 12, 2020 •

edited

Loading