Hot answers tagged

17

Prefer C++ headers (such as <cstdio>) to the C compatibility headers (<stdio.h>). The C++ headers define identifiers in the std namespace where we want them. It seems that this header isn't even used here, so we can omit it completely. std::size_t is misspellt throughout as size_t. This commonly happens when writing on a platform that declares ...


15

Avoid excessive memory usage At each step of the recursion, you allocate two arrays that together are as large as the input. That means that your algorithm uses \$\mathcal{O}(N \log N)\$ space. It should be possible to rewrite your code so you only use \$\mathcal{O}(N)\$ space without substantially changing the algorithm. But this also brings me to: In-place ...


8

Use unique_ptr instead of new[]/delete[] std::complex<double>* input = new std::complex<double>[N]; ... delete[] input; This line can be replaced by appropriate use of std::unique_ptr: auto input_storage = std::make_unique<std::complex<double>[]>(N); // #include <memory> std::complex<double>* input = input_storage.get()...


5

Regarding your program's efficiency, remember that premature optimisation is the root of all evil. Looking at your program, I can tell that your program will be IO bound - meaning that the computer will spend longer waiting for the user to input the data than actually executing your program. Moreover, functions like std::cout which deal with output, include ...


4

Overview It's a mistake to be using RAW pointers you should definitely be using some form of smart pointer here. It does two things for you: It will greatly simplify your code. It will remove the bugs !!! If you have owned RAW pointers in your class then you MUST implement the rule of three. You do (have owned RAW pointers) but your don't (implement the ...


4

Profiling results Using the profiler in Visual Studio, on my machine (Intel Haswell, 4770K) I get a breakdown roughly like this: unorderedFFT 107ms rootsOfUnityCalculator 21ms bitReversal 17ms So let's improve all of those. Roots of unity The N'th roots of unity in the complex plane is a set of equally spaced points on the unit circle. Their coordinates ...


4

Don't generate all possible permutations Generating all possible permutations will be horribly slow. If you treat the \$N \times N\$ board as a sequence of \$N^2\$ values, where the values are \$1\dots N\$ with every value having equal probability, then the number of permutations is \$\frac{N^2!}{N!^N}\$. If you generate all permutations for a single row, ...


3

Here are some things that may help you improve your code. Provide a way for users to catch errors The cli constructor can throw an exception, which is not necessarily bad, but the lack of any other constructors make it hard to use. Consider this snippet as an attempt to use your library: cli c; try { c = cli{parameters, argc, argv}; } catch (std::...


3

While formatting is a matter of preference, you shouldn't stray too far away from common ground. Consider moving your opening and closing brackets back, so: int main() { return 0; } instead of int main() { return 0; } Message_type Make a stand, either CamelCase or snake_case, not some weird offspring. while (running) ...


2

Understand argument passing In the code below int sol(int i,int j,vector<vector<char>>v,int h,int w,vector<vector<int>>dp){ you pass all arguments by value. This means that the compiler will generate deep copies of all the parameters. As you're calling this function repeatedly, this is very expensive for the vectors. However this ...


2

Input and output count enum InputOutputCount { NOTHING = 0, //Is this redundant? ONE = 1, MULTIPLE = -1 }; Yes, I think a pin that does not allow anything to connect to it is redundant. This could be replaced with a simple bool that indicates whether multiple connections are allowed or not. Note that it doesn't make sense to restrict the number ...


2

Very cool. This is like a bespoke, simplified version of Boost.ProgramOptions. It could definitely be useful. Unfortunately, the restriction to C++14 (and disallowing any 3rd party libraries) would make it completely useless to me, and most people nowadays, but maybe some people out there are still stuck using C++14. Let’s dive in: struct parameter ...


1

Avoid storing data unnecessarily Your class stores the start time, stop time and frequency in member variables, however you only need to store the start time: class Timer { LARGE_INTEGER start_count; public: // Query performance counter at the start of a block of code void start() { QueryPerformanceCounter(&start_count); } //...


1

I feel this question is more suited for stack overflow, but I'll answer it anyway. In c++ the preferred way to do this is: std::array<std::array<int, 10>, 10> arr = {}; // Without the 0 Also remember that static variables are zero-initialised anyway, so you wouldn't need to do that. Because this is code review, I would suggest using a typedef to ...


1

In addition to Martin's thorough answer, one small point: The header uses std::string but does not include <string>. Conversely, it includes <iostream> which it does not use (and which can be moved to the implementation file).


1

Prefer using getaddrinfo() You are using InetPton() to convert an IPv4 address to a sockaddr_in. However, this function has several drawbacks: It does not perform DNS lookups, so you can't use a hostname to connect to a server. You have to know up front which address family to use. It doesn't fill in all of a sockaddr_in, it only provides you with the ...


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