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.

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

15 setnx #67

Merged
merged 5 commits into from
Feb 10, 2023
Merged

15 setnx #67

merged 5 commits into from
Feb 10, 2023

Conversation

Henrik-Bengtsson
Copy link
Contributor

Add SETNX method in HazeDatabase and execute command in main.

@Henrik-Bengtsson Henrik-Bengtsson linked an issue Feb 9, 2023 that may be closed by this pull request
Copy link
Contributor

@safstromo safstromo left a comment

Choose a reason for hiding this comment

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

Looks good!
the only change id like to see is the server responses (:0\r\n) etc. Make them a constant with a name for easier reading 👍

safstromo
safstromo previously approved these changes Feb 10, 2023
@Henrik-Bengtsson Henrik-Bengtsson added the feature Marks a pull request as a new feature label Feb 10, 2023
xCandyFun
xCandyFun previously approved these changes Feb 10, 2023
Copy link
Contributor

@xCandyFun xCandyFun left a comment

Choose a reason for hiding this comment

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

Everything looks good.

Copy link
Contributor

@williamDenStore williamDenStore left a comment

Choose a reason for hiding this comment

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

Looks good!

@sonarcloud
Copy link

sonarcloud bot commented Feb 10, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Henrik-Bengtsson
Copy link
Contributor Author

Henrik-Bengtsson commented Feb 10, 2023

For test from client I have used Jedis from https://github.com/kappsegla/Build23/tree/redis

@MikaelEdwartz
Copy link
Contributor

Would it be possible to add some functionality to retain more than one value?
In this code in the getValueIfExists method it only collects value if the size is == 3

In issue #14 we need to add delete functionality, which can have n parameters.
Otherwise we would have to add the functionality later and possibly cause a merge conflict.

Or maybe add an extra if statement saying something in the lines of if(DEL) then save all the keys to a string which we then can split into each single keys for the HazeDatabase.del method?

Copy link
Contributor

@cchriss123 cchriss123 left a comment

Choose a reason for hiding this comment

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

Looks god. A bit more easy to read after last commit.

Code looks fine to me.

@kappsegla kappsegla merged commit 33dcb47 into main Feb 10, 2023
@kappsegla kappsegla deleted the 15-setnx branch February 10, 2023 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Marks a pull request as a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SETNX
7 participants