26

The algorithm This algorithm creates every combination of ships and tests them one by one, including combinations that could be seen to be useless by the first choice made. For example, if the first choice is to assign a unit of weight 60 to a ship with capacity 50, then it no longer matters what the rest of the content of arrayWithGeneratedNumbers will be, ...


23

This is some very nice-looking code, and it looks like it is working. My criticism falls into the following topics: Discussions about style, naming, tools used etc. I assume you have consciously settled on a certain style, but some aspects strike me as so unusual that I would like to talk about them. I too, like to overengineer. But there are some parts of ...


22

The fact that this was an interview question changes how I look at the code. The interviewer was almost certainly looking for one answer: "union-find data structure" or "disjoint sets data structure". You were being judged by whether you said those magic words within the first few seconds, could come up with something similar on your own, could come up ...


22

Recursion is something that is often "unwrapped" to become loops, but the same is true, in the other direction. Almost all loops can be implemented as recursion. Using your "trick" of throwing an exception to act as a conditional, it would be simple to turn all your code... this code: public static final String verseText = "On the %s day of Christmas,\...


21

Usability I have a remark about defining recursive functions. You don't want consumers of your API to think about the intermediate variables and their initial value. The consumer does not care you are using recursion internally. Make your recursive function private. private static int maxPlayer1(int[] a, int currSum, int sum, int start, int len) And ...


20

Your method is lying. Not only it's displaying the digits, performing every Console.Write operation that needs to happen, it's also performing the "digit-splitting" logic. It's more work than what its name says. For a simple coding exercise it's without consequences, but in larger projects if this is a coding habit you have, it can mean much bigger problems....


20

Use container where appropriate Right now, you have numbs defined as an array. If you, instead, defined it as an std::array or std::vector, it would make your life quite a bit easier (e.g., you wouldn't have to compute its size as (sizeof(numbs)/sizeof(numbs[0]) because its size() member would tell you the number of elements directly. Avoid magic numbers ...


19

Python 3.x bug Did you know that your program outputs 1 for the number 39 if executed on Python 3? This is because of the / division operator on this line: return get_digits(num / 10, digits) The meaning of / in Python 3.x was changed (PEP 238). Switching to // would be a quick fix. Improving the solution I was thinking I might consolidate these ...


19

Your code seems to be working and trying to use OOP is a nice touch. Let's see what can be improved. Style There is an official standard Python style guide called PEP 8. This is a highly recommended reading. It gives guidelines to help writing code that is both readable and consistent. The Python community tries to follow these guidelines, more or less ...


18

There are three reasonable responses here: yes, your recursion code can be improved for performance. yes, part of that improvement can come from sorting the data. yes, there's a way to refactor the code to not use recursion, and it may even be faster. Bearing that in mind, this answer becomes 'complicated'. Basic performance improvements for current code: ...


17

Use a better container The main issue is that std::vector<int> is a bad container for storing a set of ASCII characters. Every vector allocates on the heap, and an empty vector is likely 24 bytes in size. So this is huge waste. Why not use a container designed to hold a series of ASCII characters, like... std::string! Do you need to store all ...


16

Recursion is usually not a favoured strategy in Java due to concerns about stack overflow and function call efficiency. However, we can still review FizzBuzz as an exercise in recursive programming. The first observation I have is that recursive solutions usually start by checking for the base case first. I recommend that you stick to that pattern. As a ...


15

Since you are asking what are the cases to consider, I'll give you a very simple one. Following that, there is a further refinement to achieve a final solution as well as a few comments to consider. Trading space for speed using memoization Definition: https://en.wikipedia.org/wiki/Memoization Basically, we want to keep a look-up table of Fibonacci ...


15

I am required to write a program that prints the Twelve Days of Christmas without loops or conditionals... Usually, that's code for using try-catch to purposefully let parts of your code fail (e.g. when determining an array index), and then remediate that in the catch. Makes for a 'fun' academic exercise such as this, just bear in mind this should strictly ...


15

C++ Vs C As has been said in the comments, your code reads very much like it's been written in C. You're using C functions like printf instead of the more C++ cout and you haven't broken your logic down into classes etc. Commented out Code Commented out code creates noise that distracts from the rest of the code. If you want to have conditional ...


15

The first thing that will help is to switch this from a recursive algorithm to an iterative one. This will prevent the stack overflow that prevents you from solving 25x25, and will be a bit faster to boot. However to speed this up more, you will probably need to use a smarter algorithm. If you track what numbers are possible in each square, you will find ...


14

I believe the parameter names val and array are switched around. An array shouldn't be a single variable, and a value shouldn't be an array. Otherwise, the names themselves are okay. It's preferred to index through a C-style array with an std::size_t, especially if the array size is larger than an int. This could be helpful if you'll be working with very ...


14

For all practical purposes, recursion is a bad idea for this problem, mainly because String.substring() is an expensive operation, especially since Java 7. Let's assume that you're doing this just as a fun educational exercise. Single return statements are overrated. There's no practical advantage to forcing your code to have just one return for its own ...


14

One of the nice things with recursion is that you can use it for argument handling. Where you have a conditional to handle the b > a condition, you could instead use recursion..... def gcd(a, b): """Returns the greatest common divisor of a and b. Should be implemented using recursion. >>> gcd(34, 19) 1 >>> gcd(39, ...


14

Write useful comments carryingCapacities = new ArrayList<Integer>(); // create arraylist for the carrying capacities of ships strengths = new ArrayList<Integer>(); //create arraylists for strengths and weights of units weights = new ArrayList<Integer>(); //create arraylists for weights sc = new Scanner(new File(input)); // load my input ...


13

From a brief glance, I can see a number of issues with the code. I'm sorry if I am overly blunt, but it's better that you hear it from me than a recruiter. In no particular order: Use the language. Structure your code better. Consider using more classes to represent the board. Don't write a sudoku-solving program. Make it a tiny sudoku-solver library, and ...


13

There are a few things I see in here that I would suggest could be different. I don't like that the logic for determining whether a cell is a sink is on both Rainfall and Cell. In fact, both classes have the method called is_sink... My preference would be to move the logic on to the Cell, which already knows how to calculate it's neighbours.... and then ...


13

While your solution appears to work, and I cannot see any bugs, there are a number of best-practices that you are not employing in your solution. It is alsmost as if your code works by coincidence, rather than by design. There are a number of things that look like desperate attempts to make it work... and, like duct-tape, they work, but they are not pretty. ...


13

Your first upper bound is buggy return value + (v * (self.capacity - w)/float(w)) should be: return value + (v * (self.capacity - weight)/float(w)) If w_i << W you are overestimating the upper bound by heaps, which causes your branch and bound to never "bound". Your second relaxation is not an upper bound I'm assuming your linear relaxation is ...


13

This is slightly irrelevent but you can improve your code quite easily by removing recursion. def gcd(a, b): while b: a, b = b, a % b return a Some might argue that this reduces readibility but with a name gcd, I find it hard to believe, especially considering the fact that Euclidean algorithm is quite popular.


13

Rather than memoizing the applicable portion of Pascal's triangle, you could calculate the value much faster either along the row or along the diagonal. Let \$n\$ represent the row and \$k\$ represent the column. We know that \$\tbinom{n}{0}=1\$ and \$\tbinom{1}{k} = k\$. To compute the diagonal containing the elements \$\tbinom{n}{0}\$, \$\tbinom{n+1}{1}\...


13

Naming There are .NET Naming Guidelines which state that methods should be named using PascalCase casing. You haven't done this for buildString(). The code The InnerXml property returns a string hence there is no need to call ToString() on the property. Because all methods are public you should do proper parameter validation. At least you should check if ...


13

The simpler code would look something like this: #include <string> #include <algorithm> #include <iostream> int main() { std::string s = "abcde"; while (std::next_permutation(s.begin(), s.end())) std::cout << s << "\n"; } If you wanted to do something on the same general order as a Python generator, you'd use ...


13

#include <iostream> #include <optional> #include <string> #include <string_view> #include <vector> #include <utility> #include <exception> #include <fstream> #include <cctype> #include <algorithm> I find it helps if I keep my includes in alphabetical order - that makes it easy to quickly check ...


12

I am not an expert at scala, but this works for me def pairUp2( list1: List[A], list2: List[B]): List[(A, B)] = { (for{ a <- list1 b <- list2 if a.serial == b.serial } yield (a,b)) } val pairs = pairUp2(l1, l2) pairs: List[(A, B)] = List((A(0),B(0)), (A(1),B(1)), (A(2),B(2)), (A(3),B(3)), (A(4),B(4))) The for comprehension, ...


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