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

I presume lines is a Collection of some sort. One option that has slightly less of a smell (although it still is odoriferous), is to use an iterator, which will essentially do the same work, but will be more readable: for (Iterator<String> it = lines.iterator(); it.hasNext();) { writer.write(it.next()); if (it.hasNext()) { writer....


22

A special case length < 3 seems like a bug. A string aa is a palindrome, and one could successfully argue that a single-letter string is also a palindrome. A boolean isPalindrome is redundant, and forces the code to test the effectively same condition twice. An early return is perfectly fine here. A traditional idiomatic (and arguably more performant) way ...


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 ...


21

I don't like the multiple different assignments inside the operator. They currently all assign to gcd but you need to study it in detail to work that out. I would make the assignment explicit and the ternary operator return the correct value. The other thing with ternary operator is that they are not trivial to read. So use white space to try and make it ...


18

Why do you use || between the three conditions only to then make a decision about the conditions seperately? Why don't you write it like this: int recursive_euclidean( int num1, int num2 ) { if( num1 == num2 ) return num1; else if( num1 < 0 ) return num2; else if( num2 < 0) return num1; else if( num1 > num2) ...


15

There are some things to be said about your C version as well, but since you explicitly asked about the C++ version (and also because my C-knowledge is not that great), I will leave those for somebody else to comment on. General Hints and Tips You are passing possiblePalindrome by value where a pass by const& would be much more appropriate since you ...


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)); }


14

No, not \$O(n)\$. Not even close. I'll get back to this later though. First, the code: It's a little difficult to tell what your loop logic is. curr is getting incremented each time, but i isn't... whereas typically we'd use i as a loop index. I'd propose using i as the loop index, and then keeping a humble number count named count (or num_humbles or ...


13

This case is usually concerning joining strings. There's a method for that in Apache Commons Lang: StringUtils.join(lines, "\n"); Also here's a pattern that can be used with a foreach loop StringBuilder buf = new StringBuilder(); for (String line : lines) { if(buf.length() > 0) { buf.append("\n"); } buf.append(line); }


13

A minor thing first, in Java it is recommended to use String[] clubName rather than String clubName[]. It will have the same effect, but writing String[] is clearer. You ask for a loop, so I'll try to explain how to reach the conclusion... Regular, normal, for loops looks like this: for (int i = START; i < MAX; i++) { // code } In this case, as ...


13

I've seen a lot of answers here over-generalizing. You're dealing with a std::string, so you can just do: bool IsPalindrome(const std::string& s) { return std::equal(s.begin(), s.begin() + s.size() / 2, s.rbegin()); } You don't need anything fancy to find the midpoint. Just add the size/2. As others have mentioned, take the string by const std::...


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 ...


12

You can use LINQ's Zip method. As usual, it's a bit slower than manually written code, but unless this is in a hot spot it rarely matters. The cost of Console.WriteLine vastly exceeds the cost of LINQ. foreach(var pair in collection1.Zip(collection2, Tuple.Create)) { Console.WriteLine("{0}, {1}", pair.Item1, pair.Item2); } You can replace Tuple.Create ...


11

I would have completely agreed with the idea that iterative should be faster than recursive, I have seen proof to that effect many times in other languages. It is however harder to read. Here however, in JavaScript perhaps it is not, at least the benchmarks seem to suggest that the necessary object creation outweighs the function call overhead. To that ...


11

Potential bug: What if the array contains a single element, and it is even? if(a.length <= 1) { return 0.0; } That's what happens. Easy fix, change to: if(a.length == 0) { Your indentation is slightly screwed up, but that might just be a copy-paste error. double even should be int even. You could use a for-each loop, for (int i : a) { if(i ...


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 ...


10

If you're using Java 8, IntStream can be used to solve the problem along with its filter and count methods: public static double percentEven(int[] values) { IntStream evens = IntStream.of(values).filter(x -> x % 2 == 0); return (double) evens.count() / values.length * 100; } It's not a big win in this particular case, but I think it is a bit cleaner....


10

The best way to discuss performance is by measuring it. I suggest you use BenchmarkDotNet and never roll your own solution. Onto your question: it is safe to assume that LINQ will in most cases be slower than the iterative approach. Not only does it allocate more (for example through lambdas) but it's also more expensive in runtime because of interface ...


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 ...


10

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(...


10

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(...


9

I know you are doing this as an exercise, and personally I like the recursive function. But just as an alternative, there is also the much forgotten TreeWalker API. Browser compatibility Supported by IE9+, FF2+, Chrome 1+, Safari 3+, Opera 9+ Javascript var treeWalker = document.createTreeWalker(document.getElementById("list"), NodeFilter.SHOW_ALL, ...


9

Correctness You can't support remove() because your iterator must always be at least one step behind the iterator being wrapped. Design You twist the code to make a null filter equal to a filter that accepts everything. Don't do that. Write a filter that accepts everything and use that instead. Your class should be final. It's not designed to be ...


8

I see three basic options you have, to structure progress feedback: yield a callback function inline, hard-coded feedback Using yield, as you've shown, is one approach. Ensure you are familiar with Python's implementation of yield and iterables and generators, since several languages share the yield keyword superficially but have subtle implementation ...


8

You can do it by just removing the last seperator when you are done: CharSequence concatSep(Iterable<?> items, CharSequence separator){ if(!lines.iterator().hasNext()) return ""; StringBuilder b = new StringBuilder(); for(Object item: items) b.append(item.toString()).append(separator); return b.delete(b.length() - separator....


8

Iterating backwards was smart, but, yes, there is a much simpler way to do this using a list comprehension – this is how one typically filters a list in Python. filtered_words = [word for word in word_list if word != user_input] Note that this creates a new list object rather than altering the original word list in place. Your current code isn't terribly ...


7

One elegant way to handle this is to parse the input into a more usable structure before transforming it to HTML. For example, each section could be represented as: private class Section { final String headline; final List<String> contents = new ArrayList<String>(); Section(String headline) { this.headline = headline; } String ...


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