The Stack Overflow podcast is back! Listen to an interview with our new CEO.

# Tag Info

50

I'd like to see if this is a reasonable solution, or is just plain terrible. I wouldn't say it is "terrible" - mostly because it works and doesn't appear to be very inefficient. However, there are some improvements that can be made. use strict equality comparison - i.e. === when comparing values. That way it won't need to convert the types. Style Guide ...

24

One of the problems is that the case where you check i % 15 (i.e. i is a multiple of 3 and 5) is unnecessary. You have the concept of 3 and 5 repeated, and the concept of Fizz and Buzz repeated. This is not currently much of a problem but suppose someone asks you to extend your program to print "Jazz" when i is a multiple of 7. Using your current strategy ...

22

You should have a look at Loop like a native!. Explicitly iterating over the indices is usually not the best way to do something in Python. By directly iterating over the string representation of n you won't even need the second argument (and you should remove it if the assignment/defined interface would not require it). This will help you a lot later, when ...

15

I think it's fine as is, though some folks like to return early instead of using a series of if..else. For example: function calc(i) { if(i % 15 == 0) return "FizzBuzz"; if(i % 3 == 0) return "Fizz"; if(i % 5 == 0) return "Buzz"; return i; } for(var i=1;i<=100;i++) { console.log(calc(i)); }

13

If you are just doing a single-shot printing of a container, just print the first element and then print delim-value pairs. This reduces the iterator requirements to being equality comparable, dereferenceable, and incrementable. if (!vec.empty()) { // invert to return immediately if this is a function auto first = vec.cbegin(); std::cout << *...

13

As mentioned in another answer, iterating with indices in Python in most cases is a bad idea. Python has many methods and modules to avoid it. For your problem Python has powerful decimal module to work with decimal numbers. Here is how it can solve your problem: from decimal import * def count_even(number, length): return sum( d % 2 == 0 ...

11

Some suggestions: The trailing slash in the mkdir command is redundant. $(…) is preferred over backticks for command substitution. Why use seq in one command? They both do the same loop, so you might as well use {1..100} in both places. Semicolons are unnecessary in the vast majority of cases. Simply use a newline to achieve the same separation between ... 11 functools.singledispatch functools library includes the singledispatch() decorator. It lets you provide a generic function, but provide special cases based on the type of the first argument. import functools import random @functools.singledispatch def shuffle(arg, order): """this is the generic shuffle function""" lst = list(arg) return type(... 11 This is a fun problem. I've taken the liberty of writing an alternate implementation, which I will describe below. Note that I've not focused too much on performance at all, only on simplicity and good structure. Suggested from typing import Iterable N = 11 class MBI: letters = tuple('ACDEFGHJKMNPQRTUVWXYZ') numbers = tuple(str(i) for i in range(... 10 Since you are dealing with digits, you at some point must pick a base (in this case base 10). So converting the number to a string is not a bad way of getting the digits. I would keep it simple, and just check each digit and see if it is one of the digits you want. # Incorporating the other suggestions to loop over the chars directly rather than using an ... 7 I think your solution 2 is heading the right direction here. What I consider it's advantages over solution 1: It is much more readable It clearly shows that every member of *args is treated the same. You might want to generic-ify it a bit more to handle more types. For example, the following has a good chance of also handling custom container types: ... 7 While I do agree with others that Solution 2 is more readable with some improvements, there are also a few improvements that can be done on Solution 1. It is unnecessary to construct lists from iterables (e.g., generator expressions) when all that is needed is an iterable. For example, _args = [arg if type(arg)!=dict else arg.items() for arg in args] ... 6 First off, this is buggy. If one of the input arrays is empty, you'll get an ArrayIndexOutOfBoundsException - and even if you correct that error, the iterator will stop if it encounters an empty array. Consider using proper tests with multiple scenarios, especially with edge cases, in order to catch errors like that. Then I get two warnings from Eclipse (... 6 My main concern is the use of cw as a "persistent pointer". Specifically, might people be confused when they see for code in cw? No. Instead, you can just remove the line cw = iter(code_words) as long as it's a native iterable. "Persistent Pointer" isn't a thing in python, because all python knows are Names. What should be the typical best practices in ... 6 Explicit is better than implicit. Simple is better than complex. Readability counts. The easiest way to make it faster would be to simply convert it into code: def generate_mbis() -> Iterator[str]: """ Generate MBIs, starting with 1A00A00AA00. An MBI is a string with the following rules: [1-9] # No ... 6 As @AustinHastings pointed out, the itertools.product solution below doesn't work right unless you start at the beginning. So here's an alternate solution: DIGIT = '0123456789' LETTER = 'ACDEFGHJKMNPQRTUVWXYZ' DIGLET = DIGIT + LETTER def mbi_gen(start=None): pattern = [DIGIT[1:], # leading non-zero digit LETTER, ... 5 Expanding on Michael's answer, you could also create an object with all the words you want, with the key being the number your input must be divisible by, to make this more dynamic and a bit more future proof (A lot easier to add a new entry to the object versus adding more lines of code), and the object itself can also be dynamically generated. ... 5 Don't repeat yourself You use x/size and x%size a lot. You can easily assign them to local variables, and your code becomes much better readable. x is not the best name for an index, consider using i. int x = i % size; int y = i / size; if(x == size-1 && magicNumber-row_sum[y] <= squareSize && usedNumbers[... 5 I'm not sure that elegance should be your first concern here, because the algorithm should be very simple and the code short however you write them. I/O operations will have a much greater impact on performance than the lay-out of your loop, so you shouldn't worry too much about speed either. Generality, I believe, is paramount in this case. That's why I ... 5 There are a few quick improvements you can make. First, always remove as many things as possible from for loops. In this case, the date formatting and the open file lines can be removed. Dates. Format the dates in your dataframe before the first for loop with something like df['Date'] = pd.to_datetime(df['Date']).dt.date Notice I’m converting the datetime ... 5 You have access to the stack array directly so you can copy the unsorted stack it to a new stack and sort it in place using Array.sort "use strict"; function sortStack(stack) { const sorted = new Stack(); while (!stack.isEmpty()) { sorted.push(stack.pop()) } sorted.storage.sort((a, b) => a - b); return sorted; } or "use strict"; ... 4 I agree with Mike Brant's comments that it would be better to have the complete code posted for a broad review but that likely won't happen- if you do want to do so then I advise you to do so in a new post, since editing your post might invalidate the advice below. Nonetheless i see a few idiomatic PHP aspects that could be improved. The code could be ... 4 In SICP the authors note: Count-change generates a tree-recursive process with redundancies similar to those in our first implementation of fib. (It will take quite a while for that 292 to be computed.) On the other hand, it is not obvious how to design a better algorithm for computing the result, and we leave this problem as a challenge. And while ... 4 It looks like you have incorporated much of the feedback from AJNeufeld's answer to your previous question and the code looks better. The indentation is a little inconsistent though - for example, some lines are indented with two spaces, some four (or one tab), and then in some spots eight spaces/two tabs (e.g. the 10th line of solve()). Make the indentation ... 4 Inside of a function call, avoid echoing. By hardcoding echoes, you prevent the "silent" usage of the function. It may be necessary in the future to present the output in more than one format, so use a return inside the function declaration and perform the echo on the function call. Pay close attention to psr-2's guidelines on control structures. They ... 4 Review Function hasAceInHand could be boiled down to a one-liner using array.find. It returns the first element of the array that passes the condition x => x.face === "A" provided. const hasAceInHand = (cardsOnHand) => { return cardsOnHand.find(x => x.face === "A") != null; } Function countHandValue iterates the items, and calls the other ... 4 The point of this answer is to illustrate that the most efficient way to do things in other languages can be different from Python, and in this case is. IDK if this different low-level perspective on the problem is a good code-review answer, but putting it here in case people find it interesting. In an efficient compiled language, all that casting to ... 4 I think RangeCollection would be a better name than Ranges. To my mind, the RangeIterator is doing too much. It's iterating over a collection of ranges and over each range in the collection. To me it would make more sense for the Range class to have its own iterator and the RangeCollection its own iterator 4 Using GNU Parallel you code will look something like this: #!/usr/bin/env bash export script="path_to_python_script" doit() { i="$1" j="$2"$script -args_1 "$i"$script -args_1 "\$i" -args_2 value -args_3 value } export -f doit parallel --resume --results data/file_{1}-{2}.txt doit ::: {1..100} ::: {1..100} In your original code if one ...

4

A few notes: Firstly, I would add some docstrings to each function to document what the expected behaviour is. In select when you're not doing any mutations (i.e. under the * case), it seems you could just return self. Is there any reason to make a new copy? In select, where, astype instead of creating a temporary dict, you could use a dict comprehension ...

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