29 votes
Accepted

Python - Tkinter - periodic table of chemical elements

I hope you enjoy this bit of code. I did! Your symbols, keywords and values should have capitalised variable names since they're global constants. However, life would be easier if your symbols were ...
Reinderien's user avatar
  • 60.8k
26 votes

Python - Tkinter - periodic table of chemical elements

Don't optimize the wrong thing You have an excellent review already, so I'll comment on a narrow topic: cramped code layout. For example: ...
FMc's user avatar
  • 12.7k
23 votes
Accepted

Implementing a Stack with Templates and Smart pointers

Readability Please put a space after the include and before the < ...
Martin York's user avatar
  • 94.6k
23 votes
Accepted

A vector implementation

Design The main thing about a vector is not constructing its members until they are put into the container. The vector you have constructs all the members immediately which can be very expensive if ...
Martin York's user avatar
  • 94.6k
21 votes
Accepted

Modern C++ Singleton Template

Okay so first of the obligatory Singletons are bad practice so you probably shouldn't make it easy to write bad code. Ignoring the fact that the class probably shouldn't exist at all we can look at ...
Emily L.'s user avatar
  • 16.6k
16 votes

2D counterpart of std::array in C++17

Let me collect a couple of thoughts here. Aggregate initialization currently works like this: array2d<int, 2, 2> a{{1, 2, 3, 4}}; but wouldn't it be ...
lubgr's user avatar
  • 956
15 votes

Modern C++ Singleton Template

Singletons make it hard to test your code, and in my job I'd reject this at review for encouraging the development of untestable features. That said, I'll continue reviewing despite that. No need for ...
Toby Speight's user avatar
  • 74.5k
15 votes
Accepted

"contains" function for STL containers

If you like new features, and even experimental features, you can make your code a lot cleaner. Concepts A lot of those arcane SFINAE techniques will be obsolete once ...
papagaga's user avatar
  • 5,747
14 votes
Accepted

Vector implementation, which received unhelpful negative feedback from professor

I understand that some of this class is predicated by your professor; However I will review the code as a whole and you can then choose what can be changed and what cannot be changed. Interface/High-...
Emily L.'s user avatar
  • 16.6k
13 votes
Accepted

C++ reduce repetitions of code for similar classes

Yes, there is. In fact, a very nice way to solve this code redundancy issue comes to mind if you consider that std::string is a just type alias for ...
Ben Steffan's user avatar
  • 5,228
12 votes

C++11 Custom Vector Implementation

This is not a bad attempt at making a vector-like container, except it is lacking support for const and makes unnecessary copies (see below for a more detailed ...
G. Sliepen's user avatar
  • 54.2k
11 votes
Accepted

Templated CSV file parser

#include <iostream> You don't seem to need this include. size_t _width; size_t _height; These should both be ...
indi's user avatar
  • 11k
11 votes
Accepted

C++ math vector template

A few issues: Functions defined in the class in the header are inline by default, so there's no need to use the keyword everywhere. Use ...
user673679's user avatar
  • 11.6k
10 votes

C++ Stack Implementation Using Templates and Linked List

If you have a destructor you should also define the copy constructor and copy assign + the move variants. Otherwise the compiler will generate its own (incorrect) copy and move facilities which will ...
ratchet freak's user avatar
10 votes

"contains" function for STL containers

Rather than writing your detection from scratch, use the detection idiom, either in the compiler's header if you have it, or a copy of the published definitions otherwise. ...
JDługosz's user avatar
  • 11.3k
10 votes

"contains" function for STL containers

Wrong usage One shouldn't be afraid of manual SFINAE, but one should also use it appropriately. I'm in no way professional in this question, but guidelines are roughly the following for SFINAE and ...
Incomputable's user avatar
  • 9,444
10 votes

YAVI (Yet another vector implementation)

As a learner, I think you have done a great job. Here's some suggestions: General Don't use multiple public: labels. It seems your intent is to split the ...
L. F.'s user avatar
  • 9,508
10 votes
Accepted

Checking if a value is in a container

Neat idea. ...
indi's user avatar
  • 11k
9 votes
Accepted

Concurrent FIFO in C++11

I'm in a bit of a hurry so I'll only touch on the big high level stuff that I noticed. Abuse of SFINAE I think that the way you are designing two different use cases into one template class and ...
Emily L.'s user avatar
  • 16.6k
9 votes

A vector implementation

Implementation There is a lot of code duplication. Maybe introduce helper functions (in an internal namespace if you don't want to pollute the outer one)? ...
hoffmale's user avatar
  • 6,468
9 votes

Template Matrix Class: implemented some basic functionalities

Overview Potentially not very efficient. I have seen other Matrix libraries not do the actual computation until the value is needed. So what is passed around is a Matrix wrapper that contains ...
Martin York's user avatar
  • 94.6k
9 votes
Accepted

Generic Doubly Linked-list implementation in C++

Identifiers that start with a double underscore, or a single underscore followed by a capital letter, are reserved for use by the implementation. Your use of __T ...
1201ProgramAlarm's user avatar
9 votes
Accepted

C++ Shared_Ptr implementation

Overview Templated classes "normally" should provide all the code at the point of usage. As a result you should probably not have a separate shared.cpp. Though some people put the ...
Martin York's user avatar
  • 94.6k
9 votes

C++ template class to calculate CRC

Here are some things that may help you improve your code. Understand the nature of templates In modern C++, putting headers in a header file and the implementation in a ...
Edward's user avatar
  • 66.4k
9 votes
Accepted

3D Inverse Discrete Cosine Transformation Implementation in C++

Use std::uint8_t instead of BYTE I see you are declaring a type alias BYTE. However, I think ...
G. Sliepen's user avatar
  • 54.2k
8 votes
Accepted

(Optionally Concurrent) FIFO

Your popped elements will not be destructed. You move them but that isn't the same as being destructed and it doesn't guarantee that memory will be released. Furthermore your container requires T to ...
Emily L.'s user avatar
  • 16.6k
8 votes
Accepted

Generic merge sort in C++ using a user-defined comparison function or the < operator by default

Avoid a copy by passing by const reference. ...
Martin York's user avatar
  • 94.6k
8 votes
Accepted

Detecting if an element is found inside a container

Note that for functions the compiler will detect the template types based on the parameters. So you can simply write: ...
Martin York's user avatar
  • 94.6k
8 votes
Accepted

`printf` improvement with C++ variadic templates

General design Currently, your function is defined to have "undefined behavior" if the number of arguments is wrong. This is sub-optimal. Checking is trivial in this case, so report the problem in ...
L. F.'s user avatar
  • 9,508
8 votes
Accepted

Generic test case templates

C++ 17 Why limit yourself to C functions? DynamicTest(OutputType (*testfunc)(InputType), OutputType (*trustedfunc)(InputType)) I would declare this as: ...
Martin York's user avatar
  • 94.6k

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