# Tag Info

## Hot answers tagged programming-challenge

9

Use built-ins alpha = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z'] This is a very verbose and error-prone way of getting all of the ASCII lowercase letters. from string import ascii_lowercase as alpha will give approximately the same result. It is a string, instead of a ...

8

In the C++ solution, the map should be from int to std::size_t, since that's the standard data type for indexes. The method signature looks strange: nums should be a const reference, and the return type should just be a std::pair. But that's probably the fault of LeetCode. Or more generally: Don't blindly assume that these coding challenges provide good code ...

6

Alignment This will be an easy win - use aligned_alloc instead of malloc. This is only guaranteed to be available in the standard library as of C11, which you should be using anyway. Exponential reallocation This: // gracefully extend buffer size nofs++; buf = realloc(buf, BUF_SIZE*nofs*sizeof(char)); reallocates with linear growth. ...

6

I'm answering about java code, your method signature is the following: public int[] twoSum(int[] nums, int target) {} In this way you are bound to create an instance of your Solution class to call the method, without modifying the internal state of your Solution object. It would be better to use static and call the method like this: public class Solution {...

5

From a short review; arr1 and arr2 are terrible names, we could call them consonants and vowels accessing sp[i] so often is not efficient, I would store that value in a variable when dealing with lists, consider going functional (using forEach, map, reduce, join etc.) sp is not a great name There is a nicer way to iterate over the characters of a string ...

5

Just to add on to the C++ answer. You're assuming that a solution exists, due to lack of break condition in the loop. That may work for Leetcode; however, that is not a reasonable assumption. If a solution doesn't exist, you're in Undefined Behavior Land by accessing the array out of bounds. You don't need return vector<int>{index, iter-second};. ...

5

The main issue is that the comparator object is passed by value. Not only from your application to std::sort(), but it's also passed by value internally in the implementation of std::sort(). This means that bag and indices get copied by value a lot. So you ideally want to generate those only once, and then have class compare store a pointer or reference to ...

5

As explained in another answer, the implementation of the standard library that you use internally passes the comparator by value. It would not have to do that, but you still would have passed it by value to sort, and it is best to avoid that copying altogether. Now, you do not need to change the structure of your program to avoid this. First, now that you ...

4

Avoid unnecessary member variables You added res and exponent as member variables. However, they are only used inside longestDupSubString() and functions called by it. You should just declare them inside longestDupSubString() instead, and pass them by reference to other functions if necessary. But see below for why these variables might not be necessary at ...

4

Use size_t for sizes Although the LeetCode question specifies that the constructor takes an int capacity, using an int to hold a size is not appropriate for two reasons: int might not be big enough to handle all possible sizes that fit into the available memory. int is signed, and now you have to deal with potentially negative numbers. Also note that the ...

3

I think the general algorithm is OK, but you have a lot of repetition in your code! Also avoid writing out the alphabet by hand when you could have Python generate it for you. Here is the code without repetition: n = int(input('Enter a size: ')) alpha = [chr(ord('a') + i) for i in range(0, 26)] for k in range(1 - n, n): j = n - abs(k) center = '-...

3

Correct me if I am wrong but I think your code in findprimes is checking every number to see if it is a factor and then checking every factor to see if it is prime. Rather than doing this I suggest you want to find all the prime factors, and then making the list of all factors just comes from all the combinations of prime factors in two subsets. When you ...

3

Seems basically reasonable. Some English nits: You said "nominator and denominator," when the English terms are "numerator and denominator." You also seem to be using the word "evaluate(s)" as a noun (something like "precipitate" in chemistry), when I think the word you meant was more like "value(s)." struct ...

3

It is doubtful that the program is so long that it can't all be included, but you have made an effort to comply with the Code Review guidelines. Just be aware that comments such as // more logic here or // ... will sometimes get the question votes to close. Complexity You're a Java programmer so I'm going to assume you understand object oriented programming ...

3

I'm not sure if online courses even go into this, but this is the exact opposite of what good code should look like. Unless there is a dire performance requirement (which is rarer than you think) the aim is always to make your code as readable (clear/stupid/simple) as possible, so that: Errors have fewer places to hide It is easier to reason about what the ...

3

Methods vs. Functions None of your three solutions are object-oriented. Note: there is nothing wrong with that, in fact, the problem domain is really anemic, so there is not much to model with objects anyway. However, not being object-oriented, there is no need for objects here. In all three versions you use instance methods (or member functions as C++ calls ...

3

RE60K has already critiqued your algorithm, so I'll just leave some minor comments on the code style. These comments won't help you beat the time limit, but they are good habits to get into if you're going to take a coding interview someday, for example. Your class Solution has only public members. Classes with all public members are frequently defined as ...

3

Code Review Your code is a little hard to read. You should have a blank line after the solution body, to separate it from the mainline code. This code is hard to describe, document, and debug: l = map(int, input().split(' ')) print(solution(*l)) What is l? How many arguments are there? If the wrong number of arguments are given as input, the problem doesn'...

3

Ensure you match the given API Using size_t for sizes is a good thing to do, however the LeetCode problem specifies the API, and you should not change it. minimumDistance() should return int. Use static constexpr for compile-time constants Use static constexpr instead of const for the constants. This allows the compiler to make more optimizations, and they ...

3

You can iterate once over text and short-circuit right away if any character is not unique. import string def is_isogram(text): seen = set() for char in text: if (lower := char.lower()) in string.ascii_lowercase: if lower in seen: return False else: seen.add(lower) return True

3

Template You can't rename functions; but can you change their signature? i.e. StreamChecker(vector<string>& words) { would be better as StreamChecker(const vector<string> &words) { Similarly, void insert(const std::string word) { should be void insert(const std::string &word) { The same for search. Also, the const in bool query(...

3

In addition to what Reinderien said: Move class Trie inside class StreamChecker Your class Trie is not a generic class, but rather a specialized trie implementation specifically for StreamChecker. You can move it inside class StreamChecker, so that it is clear that they belong to each other, and so that class Trie does not pollute the global namespace: class ...

2

flag as boolean If you change the flags from 'Y' and 'N' to True and False You can use boolean indexing. This should speed up a lot of things already set You check for each combination word in dest_words for word in source_words on a list of words. If the check matches, you convert to a set. The containment check would be sped up by checking against a list, ...

2

next(iterable, [ default ]) I find too many try-except block can be distracting. next() takes an optional second argument--a default value to return when the iteration finishes. Here is @Graipher's _morgan() function using the 2-argument form of next() instead of try-except. def _morgan(a, b): a = iter(a) x = next(a, None) b = iter(b) y =...

2

There is no need to read the whole string into a deque. Just iterate over the strings using iterators. The only gotcha is handling the fact that either of the strings can be empty at the beginning and that one can be exhausted before the other. def _morgan(a, b): a, b = iter(a), iter(b) try: x = next(a) except StopIteration: ...

2

You recursive solution takes numbers from [0..left] and [right..n-1] where (left+1)+(n-right) <= k So even if there are k ways to select some elements from left and others from right, i.e. (0,k), (1,k-1), ... (k,0). You look at far more a bigger sample space, in worst case, it would be $O(n^2)$. I don't think much can be done with this approach.

2

LOOP works, it's an iterative version of the algorithm. But there are other ways to implement a solution in Common Lisp. I always try to solve a problem by checking the following techniques, generally in this order : Using applicative operators (MAPCAR, MAP, REDUCE...), Tail-Recursive function, Iterative function (DOTIMES, DOLIST, DO, DO*). And then I try ...

2

G. Sliepen wrote a rather comprehensive review, I'm going to expand on one point in their review, and add 2 others. Avoid using namespace std and/or #include <bits/stdc++.h> I see you forgot to add std:: to some standard library types, implying that you have using namespace std somewhere or are using the non-standard #include <bits/stdc++.h>. ...

2

The class template provided provides the necessary public interface, anything else should be private rather than public. Therefore the variable int curr_shot_id and the variable std::unordered_map<int, std::vector<pair<int, int>>> id_map; should be declared after private:. The variable curr_shot_id should be initialized by the constructor ...

2

Pass by const reference to prevent copying and modification: int expressiveWords(std::string const& base_string, std::vector<string> const& words) ^^^^^^ ^^^^^^ bool is_stretchable(std::string const& base_string, std::string const& words) ^^...

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