# Tag Info

13

EDIT: Thanks to @benrg pointing out a bug of the previous algorithm. I have revised the algorithm and moved it to the second part since the explanation is long. While the other answer focuses more on coding style, this answer will focus more on performance. Implementation Improvements I will show some ways to improve the performance of the code in the ...

13

Your code can be simplified using a simple loop, eliminating most of the duplicated code: def game_of_cups(zipcode, rules): total_points = 0 for num, rule in enumerate(rules, 1): rule_passes = rule(zipcode) points = num + 4 if rule_passes else 0 total_points += points print(f"Rule {num} got {points} points, so total ...

11

Readability The code needs more spacing. Considering the comments, the indentation and the ifs, it's hard to read without some good old empty lines. Use brackets when using conditions, especially if you have comments above your single line, it gets really confusing. Also, the readability is increased and, the biggest factor, you'll avoid weird bugs. If you ...

10

On the logic, you should notice that the next state of the i'th house becomes state[i - 1] ^ state[i + 1] (some care at the boundaries to be exercised). Upon the closer inspection you may also notice that if you represent the state of the entire block as an integer composed of bits from each house, then state = (state << 1) ^ (state >> 1) is ...

8

One of the unfortunate things you'll notice about coding challenges is that the requirements often force you to write code that isn't ideal. I'm going to call these out, even though in this case they're outside of your control. Silly requirement: The method is named solution. This name tells us nothing about the behavior of the function. Also (although less ...

7

I think that in terms of performance you'd be hard pressed to do better than a LINQ query. Not only fast but very compact and relatively easy to understand. It could look something like this: public IList<int> TopKFrequent(int[] nums, int k) { var answer = (from int n in nums group n by n into g orderby g....

7

I won't specifically address the execution time concerns to begin with; there are other issues: Don't use a class There's no reason for this to be a class. You have one method and you don't even reference self. In theory you could remove self and mark it a @staticmethod, but really it should just be a function without a class. Reuse variables Make this ...

7

I have a few other minor thoughts, but the big one is: rather than building the lookup dictionary on each call, since it's the same static and read-only data, presumably forever, make it a class-level member and don't pass it around: private static readonly IReadOnlyDictionary<char, char[]> _LettersMap = new Dictionary<char, char[]> { ...

6

Performance The recursive function is as fast as I can think of. The iterative function should enqueue root.left and root.right instead of root and root to gain a cycle (micro-optimisation). Review I find it weird that the null node is considered symmetric IsSymmetric(null). I would throw an error for invalid input. Both algorithms are not able to deal ...

6

Here if min_coins_to_make[target] == float("inf"): return 0 return min_coins_to_make[target] you use the fact that target has the last value from a preceding (nested) loop, and that happens to be n. It would be more clear to use n directly instead: if min_coins_to_make[n] == float("inf"): return 0 return min_coins_to_make[n] Then note that the ...

6

Review LetterCombinations returns an IList<string> but I see no reason to return a modifiable collection. Consider returning IEnumerable<string> instead. A method's name should be a verb. Change LetterCombinations to GetLetterCombinations, FindLetterCombinations or PermutateLetterCombinations. You did good by creating a private function that ...

6

Now that that pesky "Not looking for the review of my code" is gone... Step 1: White space Follow the PEP 8 guidelines, specifically (but not limited to) put a space around operators and after commas: n, k = map(int, input().split()) arr = list(map(int, input().split())) # read input sequence and store it as list type for i in range(k): ...

5

We need only 5 maximum elements, for that we don't need to sort whole array. We can use alternative approach Approach 1: Use 5 local variables and keep track of minimum element in it. Approach 2: Use Min-heap and insert first 5 elements of array in min-heap, then iterate on array from index = 5, if heap-root is smaller than number in array then extract ...

5

Enumerate Instead of writing range(len()), consider using enumerate. It provides the index and the value associated with that index. It's useful in your case because, instead of having to write in_states[i], you can write value instead. This will save you from having to index the list again with in_states[i]. Docstrings You should provide a docstring at ...

5

General remarks This using namespace std; is considered bad practice, see for example Why is “using namespace std” considered bad practice? on Stack Overflow. Consistent indenting and spacing increases the legibility of the code. Use curly braces for if/else blocks even if they consist only of a single statement. Enable all compiler warnings and fix ...

4

The code is mostly readable and clear: the variable names are descriptive (for the most part - x is a little unclear) there is good use of const and let instead of var Some of the lines are a little lengthy - the longest line appears to be 117 characters long (excluding indentation): newIntervals[newIntervals.length - 1][1] = Math.max(newIntervals[...

4

Personally, I think it's excessive to have a function for each of the rules. IMO, if you only need the function in a single context, you probably don't need to make a function out of it unless it's sufficiently complex to warrant one - and even then, make sure it's "private." Because the values for the rule set start at 5 and simply increment in value from ...

4

There isn't going to be a significant difference between C and C++ when using the same algorithm. Using a while loop versus a for loop in this case will not have a significant difference either. The loop is only going to sum 5 values. The problem is the algorithm, as another answer pointed out do as much of the sorting as possible as the numbers are input. ...

4

NO: C++ is not magically slower than C. Bad C++ code is slower than good C code and vice versa. NO: For the compiler it is completely irrelevant whether you use a for or a while loop as they will all be normalized into a consistent representation anyway. Now to the actual review. Your code will not improve from porting to C as you are already not using any ...

4

Your testcases are far too simple to be certain your algorithm works. This might be an issue on the challenge's side. Both these testcases have symmetrical solutions. It's best to always rob every second layer. No test cases cover instances where the left side of the tree might follow a different pattern to the right side. Take for example the testcase: { 3,...

4

Just a modification on AJNeufeld's def game_of_cups(zipcode, rules): for num, rule in enumerate(rules, 1): points = rule(zipcode) total_points += points ... def rule1(zipcode): return 5 if (zipcode[0] == zipcode[-1]) else 0 def rule2(zipcode): a, b, c, d, e = map(int, zipcode) return 6 if (b == 2 * a and c > min(b, d)) else 0 ... ...

4

(It's me, once again, same review format) Readability There are way too many comments in your code. Comments should explain why you do something when it's not already clear in the code. In my opinion, the less comments some code needs, the better it is. Also, considering your comments are formatted almost exactly as your code is, it is veerryyy confusing. ...

4

Most things have already been addressed by IEatBagels in his answer. Compactness I believe readability could be increased by writing more succinct code. Starting from this if-else block: if (countersArr[index] < floor) { countersArr[index] = floor + 1; } else { ++countersArr[index]; } We can see that we want to assign x + 1 to countersArr[...

4

The good: Using std::size_t for indexing a std::vector. Properly using std:: where necessary. :) The bad: bug: forward_count and backward_count are passed by value into please_conform. They appear to be output parameters, so they must be passed by reference. bug: Although we warn the user when encountering an empty list, we carry on running the program, ...

4

Good to see you're not doing a using namespace std ;). Of course it doesn't really hurt in a source file (.cpp) (it does in a header file!), but even then that is still considered bad practice. My personal reason to just always type the namespace is because it makes code easier to read (you know what is part of a namespace and which and don't have to guess ...

4

Some things come to mind when I read your code. There is a nice feature on arrays in C# called CopyTo(). It gives you the possibility to copy an array without using a loop. Like so: int[] newNums = new int[n]; nums.CopyTo(newNums, 1); newNums[0] = newNums[n-1] = 1; //This line I really like. There is also something called a multidimensional Array which ...

4

The comment here int[] numbers = new int[nums.Length +2]; // we add 2 because of the question is not really useful, it requires to read the entire question to figure out why 2 is added to the number of balloons. I would suggest something like // Allocate array for all balloons, plus the two "imaginary" balloons // at positions -1 and n. which closely ...

4

Any solution which does something $k$ times will be hopelessly slow. We have to make use of the structure of the problem here. Suppose $i<\lfloor n/2\rfloor$ and write $a = A_i$ and $b= A_{n-i}$. Then each time through the list we execute $a \leftarrow a\wedge b$ and then $b\leftarrow a\wedge b$. Lets record the values of $a$ and $b$ ...

3

(Adapted from my answer to How can I make my solution to “Climbing the Leaderboard” not time out?) It is nice that you used size_t and ptrdiff_t. However, int is still directly used for the score. I would prefer an alias: using score_t = long; using rank_t = long; (Note that int is not guaranteed to have $10^9$.) The m_binary_search function is ...

3

API I assume it's part of the platform requirements, but int[] is a terrible data-structure for storing the (k, h) pair, because it could have any length and implies an ordering that doesn't really exist. This should be a small class or immutable struct. (Not a Tuple, because Tuples are structural, and therefore only solve part of the problem). Even if ...

Only top voted, non community-wiki answers of a minimum length are eligible