Submission for week 2 #1
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
You need to convert all the solution files with |
sorry... my bad. now converted files to .cpp. |
Sadman007
reviewed
May 6, 2022
Comment on lines
+8
to
+21
| int strStr(string haystack, string needle){ | ||
| if(needle == "") return 0; | ||
| else { | ||
| int haystackLength = haystack.length(); | ||
| int needleLength = needle.length(); | ||
| for(int i = 0; i <= (haystackLength - needleLength); i++){ | ||
| if(haystack[i] == needle[0] && | ||
| haystack.substr(i, needleLength) == needle){ | ||
| return i; | ||
| } | ||
| } | ||
| return -1; | ||
| } | ||
| } |
There was a problem hiding this comment.
Suggested change
| int strStr(string haystack, string needle){ | |
| if(needle == "") return 0; | |
| else { | |
| int haystackLength = haystack.length(); | |
| int needleLength = needle.length(); | |
| for(int i = 0; i <= (haystackLength - needleLength); i++){ | |
| if(haystack[i] == needle[0] && | |
| haystack.substr(i, needleLength) == needle){ | |
| return i; | |
| } | |
| } | |
| return -1; | |
| } | |
| } | |
| int strStr(string haystack, string needle){ | |
| for(int i = 0; i <= (haystack.length()- needle.length()); i++){ | |
| if(haystack.substr(i, needleLength) == needle){ | |
| return i; | |
| } | |
| } | |
| return -1; | |
| } |
It does the same work, but check the readability and concise-ness.
Sadman007
reviewed
May 6, 2022
Comment on lines
+11
to
+26
| if(strs.size()) == 1){ | ||
| return strs[0]; | ||
| } | ||
| else{ | ||
| int numberOfStrings = strs.size(); | ||
| string currentLCP = strs[0]; | ||
| for(int i = 1; i < numberOfStrings; i++){ | ||
| int j; | ||
| for(int j = 0; j < strs[i].length(); j++){ | ||
| if(strs[i][j] != currentLCP[j]) break; | ||
| } | ||
| currentLCP = currentLCP.substr(0, j); | ||
| } | ||
| return currentLCP; | ||
| } | ||
| } |
There was a problem hiding this comment.
Again, is the first if condition necessary? Don't handle special cases too often in every program you write. It's not a good practice.
Sadman007
reviewed
May 6, 2022
Comment on lines
+8
to
+14
| unsigned long long getHash(string& str){ | ||
| unsigned long long hashValue = 1; | ||
| for(auto ch: str){ | ||
| hashValue = hashValue * (257 + (ch - 'a')); | ||
| } | ||
| return hashValue; | ||
| } |
There was a problem hiding this comment.
It's better not to use unsigned long long. Refer to my this post.
Sadman007
reviewed
May 6, 2022
Comment on lines
+15
to
+17
| if(numsSize == 1){ | ||
| return to_string(nums[0]); | ||
| } |
Sadman007
reviewed
May 6, 2022
week02/largest_number.md
Outdated
Comment on lines
17
to
19
| if(numsSize == 1){ | ||
| return to_string(nums[0]); | ||
| } |
Sadman007
reviewed
May 6, 2022
Comment on lines
+12
to
+17
| for(int i = 0; i < stringLength; i++){ | ||
| charCount[s[i] - 'a']++; | ||
| } | ||
| for(int i = 0; i < stringLength; i++){ | ||
| charCount[t[i] - 'a']--; | ||
| } |
There was a problem hiding this comment.
Suggested change
| for(int i = 0; i < stringLength; i++){ | |
| charCount[s[i] - 'a']++; | |
| } | |
| for(int i = 0; i < stringLength; i++){ | |
| charCount[t[i] - 'a']--; | |
| } | |
| for(int i = 0; i < stringLength; i++){ | |
| charCount[s[i] - 'a']++; | |
| charCount[t[i] - 'a']--; | |
| } |
Sadman007
reviewed
May 6, 2022
Comment on lines
+18
to
+35
| else if(s[i] == ')'){ | ||
| if(stackTop == 0) return false; // if stack is already empty return false. | ||
| top--; | ||
| if(myStack[stackTop] != '(') return false; | ||
| myStack.pop_back(); | ||
| } | ||
| else if(s[i] == '}'){ | ||
| if(stackTop == 0) return false; // if stack is already empty return false. | ||
| top--; | ||
| if(myStack[stackTop] != '{') return false; | ||
| myStack.pop_back(); | ||
| } | ||
| else if(s[i] == ']'){ | ||
| if(stackTop == 0) return false; // if stack is already empty return false. | ||
| top--; | ||
| if(myStack[stackTop] != '[') return false; | ||
| myStack.pop_back(); | ||
| } |
There was a problem hiding this comment.
So many repetitions! See this.
Sadman007
approved these changes
May 6, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is submission for week 2.