Hot answers tagged

5

The code looks well organized, and has a clear coding style. Good separation of responsibility between classes, almost no raw pointers (except for those coming from the C API of SDL of course), and no global variables. Nice! But there are still some areas of improvement: Only use SDL_assert() to check for programming errors Assertions are a tool to help ...


4

Couple of small things: Use emplace_back rather than push_back when you just have the parameters for the constructors: longList.push_back(Task{ taskInput }); // This is better written as: longList.emplace_back(taskInput); The difference between the two: push_back(Task{ taskInput });. This creates a "Task" object as an input parameter. It then calls ...


4

The Good In the class Expression you put the public interfaces first and then the private variables even though when C++ defaults the section immediately following class CLASSNAME { to private. This is very helpful for programmers that may have to maintain your code and can be considered a best practice. You use an enum to indicate the token type. Avoid ...


4

Your professor is correct. This is really hard to read, and is structured somewhat oddly. Here is an assortment of comments, left mostly in the order from top to bottom as I found them. Don't using namespace std;. This has been discussed at length on this site and elsewhere, so I'm not going to leave any additional context here. Expression::Value is a bit ...


3

Overall You don't use encapsulation. Which makes your list vulnerable to incorrect initialization and accidental incorrect modification from outside the list. You use several C based style choice rather than C++ style which make your codde harder to read. Code Review Only a list of int? class Node { public: int value; // int only ...


3

Some suggestions: Use variable names that tells the reader what they are. Dont do using namespace std; in the global namespace. Always check that <stream> >> variable actually worked or else your program will run with uninitialized variables and cause undefined behaviour if they are read. Use an unsigned type when dealing with subscripting. If ...


2

Non-const static variables Your code shows a lot of good practices: variables are always initialized when possible variables have proper names (and you stuck to a single naming scheme) no unecessary std::endl However, there is one drawback: static int random_number; That line right there is, at least from my point of view, a flaw. static variables are a ...


2

typedef R(*Invoker)(std::byte*, Args...); typedef void(*Replacer)(std::byte*, const std::byte*); Alias-declarations are easier to understand IMO: using Invoker = R (*)(std::byte*, Args...); using Replacer = void (*)(std::byte*, const std::byte*); template <typename Functor> static R genericInvoker(std::byte* f, Args... args) { static_assert(...


2

As per signature main function must return int value #include "fvpalgorithm.h" int main() { FVPAlgorithm algorithm; algorithm.run(); } Task Structure: Why is taskName public? Here taskName looks like task identifier. Will this identifier change post creation? Do you intend to support case where task is created with name X and then later changed ...


2

Your function modifies the get/put position of the buffer. I think this is a bug, and it makes benchmarks misleading. Try calling extract twice in a row -- you'll get different results. You set the put position after calling sgetn but you should have set the get position. This should be intuitive. After a read, you "undo" your reading. Fix the bug with ...


2

Code Review: I don't like this as it gives you an extra level of indirection. struct json_array { std::vector<json_value> array; }; You can simply use another name: using json_array = std::vector; This gives you a specific name for the array and removes the level of indirection. Sure this is resonable: struct json_object { std::map < ...


2

In all, this is a nice, solid effort, especially for a self-avowed "newbie." Keep up the good work! With that, here are some things that may help you improve your program. Use SFML more fully I would suggest that it would be a good idea to have the Flock class derive from both the sf::Transformable and sf::Drawable classes. Your Flock::Display() would ...


1

Declare for-loop-variables inside for instead of reusing them. So instead of: int v[501][501], i, j, m, n, o, p; cin >> m >> n >> o >> p; for (i = o; i >= 1; i--) v[i][p] = o - i; for(i = o;i <= m; i++) v[i][p] = i - o; for(i = 1; i <= m; i++) for(j = 1; j <= n; j++){ use (except you should also change ...


1

A way to shorten your code is to extract common parts — the tolower and toupper closures and general transform and transform_copy utilities, in this case: (note that I used std::locale::classic() instead of the recently installed global locale) namespace ae { struct to_lower_t { template <typename CharT> CharT operator()(...


1

What you can do is combine the two methods toLower and toUpper into one method, say transform, to which you pass the method by which to transform (std::tolower or std::toupper) as arguments. The same can be done for toUpperCopy and toLowerCopy. Passing functions allows you to be more flexible as you are now able to not only use std::tolower and std::toupper ...


1

1. You never check if head was assigned a valid pointer before dereferencing it here: int find(struct Node *head, int n) { int count = 1; //if count equal too n return node->data if(count == n) return head->value; // <--- here //recursively decrease n and increase // head to next pointer return find(head->...


1

Interface Prefer wstring_view to wstring for parameters to save one copy: ConnectionPool(std::size_t nWorkers, std::wstring_view baseUri) (Whether this is effective depends on the way http_client works.) void assignWork(int pidx, const Function& f) std::size_t pidx, I guess? (Same for other occurrences of int.) ~ConnectionPool() { } Remove this ...


1

This is a cool idea. You can improve the code by using the STL utilities that support currying/composition. For currying, there is std::bind and std::bind_front: // this is the STL way to curry functions auto demoSTL() { auto sum1 = std::bind_front(sum, 1); auto sum123 = std::bind_front(sum1, 2, 3); auto call = sum123(); return call; } ...


1

First off, I think it's worth thinking carefully about whether C++ is the best tool for this task. It certainly can be done, but you may find better documentation/more support for COM/interop from C# or VBA. Particularly for such a simple task. HRESULT hr = CoInitializeEx(NULL, COINIT_APARTMENTTHREADED); You need to check the result of CoInitializeEx and ...


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