New answers tagged

1

Having the undo in the global controller object is fine. It implies that the undo action is global. I probably would have saved the state of the robot to the queue each time a command is received. Then undo is just restoring the state to what it was at a previous time. It also makes going back 'n' moves at a time much cheaper. It is generally considered ...


2

Review ArchiveBuilder should be declared static. It does not have any non-static state or operations. Both the argument and return value of Build should be IEnumerable<>. There is no reason to use a List<>. You should not remove values from entries anyway, there is an alternative. The alternative to removing elements from entries is to use a ...


-4

If I understood your code correctly this is caused by undefined behaviour. To fix it your lambda should capture by value. (like [=]() { /*bla*/ }) Explanation auto my_helloworld = decorator(helloworld); // Create temporary FunctionPointer value on stack // Temporary will be no more after this -> Weird stuff will happen if you use it Pass as && ...


1

Your robot creator is not a factory in the traditional sense, because it never returns the robots it creates, or exposes them publicly. Generally, a factory object would do the same job as a class constructor, with the advantage of being a first-class object that can be passed around, stored in a field somewhere, and so on (without resorting to reflection or ...


2

First of all I would suggest you changing PairCurrency constructor from class PairCurrency: def __init__(self, name): self.name = name to class PairCurrency: def __init__(self, name, ratio): self.name = name self.ratio There is literally no reason to make it different. This is not a functional class but rather ...


2

Set? You have logic preventing duplicate currency pairs. The easier (and more performant) thing to do is to simply represent pair_currencies as a set instead of a list. When you add, it will automatically discard duplicates. snake_case The standard is to name methods like your last_pair - i.e., get_rate. Lookups You have a loop in getRate and editRate ...


1

Although the motivation is to create independent unit tests, it only partially achieves that result. Because the singleton is globally accessible from while it's alive, we've constrained the tests to run sequentially, which limits our ability to run more of them in a given time. There may be benefit in making the singleton be thread-local for tests (this ...


2

While your code is thread-safe, it does not scale. Consider adding a fast-path in case there is an object. There is no need for a class, a free function is enough. std::make_shared() generally coalesces the two allocations (control-block and payload), making it significantly more efficient. If you want to ensure only get_shared_singleton() can ever construct ...


2

The problem with the above code is that it doing two things with one piece of code. 1: Singleton Creation. 2: Singelton LifeSpan. I would separate these out into two separate classes. The creation is done via an abstract factory (MySingletonFactory) but lifespan is controlled via explicit factory classes that are defined for the situation (so your code ...


3

This is code review, we didn't need to see your non-abstracted class since it doesn't need reviewing. I will not do a complete review of the abstracted class, but I have a few comments and tips. First of all, your class is called CreateTable, but this class can also destroy a table. That is the opposite of what it says on the tin. I think a better name ...


Top 50 recent answers are included