New answers tagged

0

I like your code =) What it does is clear, and your variable names and spacing is correct. There is not much of an algorithm to speak of here, so instead I will focus on structure. It is always a good idea to separate inputs from actions. This can be done using a if_main guard, and clear functions. Something like this passes the first test case def ...


3

Normally I do a design review first, then review the code itself. This time I’m going to switch it around. Code review Before I get into review this code, I just have to say that the sample solution code given is… ghastly. Seriously, if someone worked at a software company I owned, and they submitted that kind of code, I’d not only fire them, I’d have them ...


1

Nice work! It's an efficient solution (linear in the length of the input string), using a simple yet quite clever logic to reorganize the letters, reasonably easy to read and well-formatted. Improving readability It's good to extract logical steps to helper methods, as you did for initializeCharMap26. You could split reorganizeString a bit further, ...


3

Code Review Your code doesn't pass all of the test cases, so by some definitions, it is not ready for a code review. However, it is close enough to working and has several code-habits that should be corrected, that I'll give you a Code Review anyway. Variable names Both count_list and word_list are nice, descriptive variable names. final is ok, but not ...


0

Try This -> var isValid = function (s = "{[]}") { const leftSymbols = []; for (let i = 0; i < s.length; i++) { if (s[i] === "(" || s[i] === "{" || s[i] === "[") { leftSymbols.push(s[i]); } else if ( s[i] === ")" && leftSymbols.length !== 0 && ...


1

__init__ has a mutable default argument. The default argument is not reset between calls to a function, so multiple analyzers that start out empty will have the same list, which could be a problem if one of them changes: >>> a1 = DrugAnalyzer() >>> a1.data.append(['G03-01', 789.01, 129.00, 0.00008]) >>> a2 = DrugAnalyzer() >>&...


0

The bug The code doesn't terminate, because the loop condition frontierRef[0] !== undefined will never be false. After the last level of the tree, newElements will have all null values, the slice will slice off all but the first element, so the loop will evaluate null !== undefined, true forever. Slow algorithm The algorithm itself is too slow to solve this ...


0

Sorry, can't add a comment so will try to make it as an answer. I have tested your code that is in Git. It returns U when there is USA passed. I understand you can't compare this kind of argument with anything but may it task requirements are not clear? Since as we know (Wikipedia too): Phrases can consist of a single word or a complete sentence Maybe in ...


1

Tiny review; functions ideally follow <verb><Subject> so wordSplit -> splitText I think it is important to realize that when you do ff(v => f(v)) you can also do ff(f), or in this case you could do wordSplit(text).forEach(count) or even textSplit(text).forEach(updateWordCount) if we used better function names ideally topn should be topN, ...


1

The code is easily readable and variables are well named. What you could do is have functions instead of lambdas, but that is just my personal preference. Also "wordSplit" is a function and would be a bit better named splitIntoWords(text) {...} From the reuse and maintenance perspective the wordSplit fells like it is a little misplaced. It is not a ...


0

I would have created something like the following which allows for better error handling and have the output buffer declared on the stack. #include <ctype.h> #include <stdlib.h> #include <string.h> #include <stdio.h> enum AbbreviationErrors { phraseIsNull = 1, bufferTooSmall = 2, noLettersFound = 3 }; int abbreviate(...


1

I'll show you what I would write first: char *abbreviate(const char *phrase) { assert(phrase); char *acronym = malloc(strlen(phrase) / 2 + 1); if (!acronym) { return NULL; } char previous = ' '; size_t len = 0; for (size_t i = 0; phrase[i]; i++) { char current = phrase[i]; if (previous != '\'' && !isalpha(previous) &&...


2

As pointed out your time calculations are incorrect. Avoid mixing units Your function is basically just calculating the months between two dates but you are working in both years and months. Rather than calculate the years then the months convert both dates to the same unit. In this case that would be months. Converting to months and back. Given int year ...


3

Avoid using namespace std - that dumps the whole of the standard library into the global namespace, eliminating all the advantages of it having a namespace. When taking input, think about how it can fail. If something that's not a number is entered, then we'll be working with uninitialized values, and all bets are off. Check with something like if (!std::...


1

else if(age >= 0 && age <= 3) cout << " The price of your ticket is: $ " << price << endl ; if( age >= 4 && age <= 64) cout<< " The price of your ticket is: $ " << price << endl ; if (age > 65) cout<< " The ...


4

We have some popular beginner problems: Don't using namespace std - this makes it harder to tell which identifiers are your own and which come from the standard library. We have namespaces to help us, and this just throws away all the benefits. Unfortunately, many tutorials present this as if it were a good practice! Error messages should go to the ...


7

Your overall code works fine, but there are some improvements. Using else if's You are comparing age against 0, 4, 64, 65 pointlessly, it can only be one. In this case we can use else if. Duplicate code Once you have determined the price, you do not need to check age again, you can directly print the output. Of course after restructuring your if-else if ...


0

Simple O(nlogn) code long long hashMap(vector<string> query, vector<vector<int>> q) { int n = query.size(); int KeyOff = 0, ValOff = 0; map<int, int> mp; int x, y; int ans = 0; for(int i = 0 ; i < n ; i++) { if(query[i] == "insert") { x = q[i][0]; y = q[i][1]; ...


3

The style in C++ is to put the * or & with the type, not the identifier. This is called out specifically near the beginning of Stroustrup’s first book, and is an intentional difference from C style. I don't like how in >> rows >> cols; needs to be repeated at the top and bottom of the loop. I think you should consider a mid-...


1

DRY. Every path through the loop ends up in dict_copy = a_dict.copy() a_lst.append(dict_copy) a_dict.clear() Lift it out out of the conditionals. You will also immediately see that the branches oop_count == len(s) become no-ops, and could be safely removed, along with now irrelevant loop_count. The loop ...


5

Consider using an array of rows The idea to use pointers into buffer is a good one, as swapping those pointers around is quite fast. However, instead of using three distinct variables to hold those pointers, consider storing them in an array: std::array<char *, 3> rows = {&buffer[0], &buffer[maxSize], &buffer[maxSize * 2]}; Now you can do ...


3

Nit-Picking: You have come to the correct place First, your whole style is more C rather C++ like. I don't see any abstraction or encapsulation. Though, C++ can include C as (mostly) a subset. The styles of how the language is used are different. This is C like more than C++ like. The things that stand out are: passing pointers (C++ prefers references or ...


3

Don’t write using namespace std;. You can, however, in a CPP file (not H file) or inside a function put individual using std::string; etc. (See SF.7.) The style in C++ is to put the * or & with the type, not the identifier. This is called out specifically near the beginning of Stroustrup’s first book, and is an intentional difference from C style. ...


2

int i, k = 3; I'm simply going to delete this line. In general, it is better to declare variables as close to first use as possible. In this case, inside the loop is possible. for(i = 2; i <= 2000000;){ For every candidate number from 2 to 2,000,000. But I can tell you immediately that about half of those won't be prime. Consider ...


1

The main problem I see is, that you don't use the number one rule in software development, which is: split the problem. When you want to know the sum of primes up to n, you basically have two distinct parts to solve: know the primes up to n sum these primes up For part 1, use a commonly known algorithm like the sieve of Erasthostenes, for part 2 do a loop. ...


3

Your "Ordered" solution doesn't need to sort the expressions. By using the fillers in the order ('+', '-', '') as you do, you already produce all strings in sorted order, as + is smaller than - is smaller than digits: >>> sorted('123456789-+') ['+', '-', '1', '2', '3', '4', '5', '6', '7', '8', '9'] Not having to sort at the end also means ...


0

I refactored your program while keeping the same logic and (IMO) made it significantly more readable. import java.util.HashSet; import java.util.Set; public class PossibleServeBreaks { public static void main(String[] args) { Set<Integer> solutions = possibleServeBreaks(2, 1); System.out.println(solutions.size()); for (int ...


1

One thing that I often forget to do as well is to simply reference the source or reason of the program. If this was an assignment, you could include that in the JavaDoc of the class. public class KBreaksInTennis { This is not very clear to the user; it's better to call it CountBreaksInTennis or something similar. static int[] displayKBreaks(int a, int b) { ...


0

If you tried logging what happens you would probably be able to figure it out :D The good part is that it has nothing to do with your program, but it has to do with JS. if (!myDict[target - ele]) { total+= memoCombinationSum4(nums, target - ele, myDict) } If myDict = { 1: 0 }, then !myDict[1] is true. Welcome to the world of pain that is JS. ...


Top 50 recent answers are included