18

Using TypeScript/ECMAScript 6 syntax it can be achieved this way. I am not sure whether it's more or less elegant compared to the original variant, but It does the job; Requires only one run; Can be further chained with map () or other functions. const [small, large] = // Use "deconstruction" style assignment [12, 5, 8, 130, ...


16

Prefer not to use pointers. Pointers do not portray ownership. So users don't know if they should pass a real pointer (to dynamically allocated objects) or if they can give the address of an object. So you have to look at the code (or documentation) to understand how to call the code. // Subject is a reference virtual void onNotify(Subject& entity, ...


12

Anything wrong with the naive ways? let large = []; let small = []; array.forEach((x) => (x > 10 ? large : small).push(x)); // or for(const x of array){ (x > 10 ? large : small).push(x); }


10

Task task; while (m_enabled) { { std::unique_lock<std::mutex> lg{ m_mu }; while (m_tasks.empty() && m_enabled) m_cond.wait(lg); } if (!m_tasks.empty()) {// there is a task available std::unique_lock<std::mutex> lg{ m_mu }; task = std::move(m_tasks.front()); m_tasks.pop(); ...


9

Welcome to Code Review! Here's some suggestions: Sort the #include directives in alphabetical order. <sstream> and <string> shouldn't be in the implementation. The constructor should not be noexcept — it allocates resources. Same for start. The destructor is implicitly noexcept, so you can omit it. In enqueue, the lambda captures &...


8

There's an existing technique which is not dissimilar referred to as "exception vomiting". Observe: void f(void(*p)()) { p(); } template<typename F> void real_f(F func) { try { throw func; } catch(...) { f([] { try { throw; } catch(F func) { func(); } ...


8

Fake for-loops and comprehensions At the beginning of your post, you say that you can't always achieve everything with looping in one line in CoffeeScript. When you say that, I assume you are taking about the each_file: each_file: (index = 0) -> length = @files.length file = @files[index] if index < length Types.read file, @...


7

First, there is a principle you should be reminded: Never deal with the error you don't know, just pop it up. Which means, if you can't deal with or don't know about the error passed in the callback, you just pass it to next callback, or throw it(A pattern of CHAIN OF RESPONSIBILITY)! For the errors you know how to deal with it, you can define a module ...


7

Starting from minor issues - please follow naming conventions (variables should be camelCased and methods PascalCased, do add access modifiers, give variables meaningful names). Your design breaks Open-Closed principle: Queue knows all the possible commands it can handle. In case when you need to add a new command you'll have to change the queue rather than ...


7

First of all, since you're using jQuery already, why not use $.proxy() which does the same thing? $('#button').click($.proxy(this, 'fooBar', true)); That's equivalent to your contextualize example, as far as I can tell. And $.proxy has the added benefit of working with functions themselves, not just their names. That means you can use it to wrap a function ...


7

It's a little difficult to give a full review without the removed null checks, but I'll give an observation: In Dispose(), you depend on null to be the value of _state to judge whether or not to call the cleanup action. That's all well and good, but you should likely then constrain the generic parameter T to be class class Usingifier<T> : IDisposable ...


7

I don't feel like you should override + and -. Fine, it makes the syntax cuter, but it makes your code much less readable for someone who isn't you. Remember, you write code so that yourself in 5 years or someone else can read it properly, not so you can feel it better right now. Follow this guideline, and you'll see that your code will be much cleaner. By ...


7

I don't think that screen space is so limited that methods can't be separated by a blank line, and separation makes it slightly easier to see scope. public delegate void NotifyCBR(); private NotifyCBR notifyCbr_ = null; Is there any reason not to use System.Action? private System.Collections.Generic.List<T> vector_; ...


6

A listener and callback mechanism is a common pattern in many places in Java. The logical place to look for examples is in the Swing API. Will get back to that in a second, but, there are two items that are useful first: Instead of using an int value to track the source of the event/Task, you should use the task itself. public void onResult(String ...


6

One thing that bothers me is, that I have both the type parameter for the class and I am passing in the class to the constructor. It seems like I should not need to pass in the same info in two different ways. The reason for why you have to do that is because of Type Erasure. Simply put, the generic class is only known during compile-time. This call is ...


6

Indeed the recommended approach for this (or at least what I would recommend) is to use callback interfaces. The most important aspect though is designing this interface. In this case you should be sure that you're passing everything you need. However, I think you can rename the method to onOK or okPressed or similar. You might also consider returning a ...


6

This is clever, but it does not work in the general case. The problem is that it requires cify to have a different template instantiation for every distinct value of the lambda that it wraps, but in reality it will only get a different instantiation for every distinct type of lambda. So if you cify two lambdas with the same type but different values of ...


6

Also, I was looking for an answer how to flatten (and automate) calls to rl.question(). In my solution I used Promises - chained - to display questions sequentially. const readline = require('readline'); const rl = readline.createInterface({ input: process.stdin, output: process.stdout }); const QUESTIONS = { action1: ['A1: Question 1', 'A1: ...


6

A few thoughts about your code: 1. Use of reinterpret_cast // Create a simple function that calls the functor this->func_ = [](void *user, TArgs ... args) -> void { TFunctor *functorPtr = reinterpret_cast<TFunctor*>(user); (*functorPtr)(args...); }; This always raises a red flag for me. I understand that the user ...


6

[code] uint64_t It should be std::uint64_t, not uint64_t for key indexes (the latter is the C version). Or better: use std::size_t, which is the standard indexing type. [code] passing arguments At the moment, all the parameter pack arguments to the function calls are being passed by value. This should instead use perfect forwarding to prevent unnecessary ...


5

I like your code, and would use it myself, some thoughts: I assume you declare url next to JSON.load because you want to declare it close to where you use it. I would still recommend using 1 comma separated list of variables on the top. lowerCamelCase is good for you, get_follows -> getFollows -> getFollowedChannels ? Comparing to 0 should be done with ===, ...


5

The approach itself (i.e. using a counter) is reasonable, but the hard-coded 2 and DoStuff limits its use. You could do something like this, to derive more generic implementation: function createCounter(count, callback) { count || (count = 1); // default to 1 (typeof callback === 'function') || (callback = function () {}); // default to no-op return ...


5

If the idea is to model this function on JavaScript's String.replace(), then there is a "bug". In JavaScript, when doing a global replace, the last argument passed to the callback is always the original unmodified string, not the string that may have had some of the replacements already applied to it, which is what you've written. The same should be true of ...


5

foreach (var number in parallelQuery) { _primes.Add(number, false); } You could simplify this to: _primes = parallelQuery.ToDictionary(n => n, n => false); public void BuyNumber(Int32 number, Action<NumberType> callback) I don't understand why are you using a callback here. The whole method is completely synchronous, so I think you should ...


5

You should not be calling it a dictionary because you changed the expected behaviour of the IDictionary interface. The MSDN clearly states that the getter of the indexer should throw an exception if the key is not found. KeyNotFoundException - The property is retrieved and key is not found. this should not be adding new elements if the getter is used. ...


5

I fully agree with TopinFrassi and t3chb0t. Some other points The MultiDictionary and the AddableHashSet are not required and complicate the solution. It is much simpler to use a dictionary with a List<TValue> as value. I understand the desire for simplifying the API to accessing the dictionary, but that can be also realized using extension methods ...


5

Don't test whether a HashSet contains a key if you want to remove it: if(list.Contains(value)) { list.Remove(value); } Instead, just straight try to remove it: list.Remove(value); The replacement is cleaner, shorter, and more efficient. What's not to like?


5

False alarm! No callback hell here. While this code can certainly be refactored a bit, I fail to see callback hell anywhere around. Callback hell is normally a term applied to the code that is executed asynchronously and with multiple nested callback functions. The nested structure is what causes a few issues: cognitive complexity (which leads to bugs and ...


4

Without knowing what you're trying to do, I'd say this is the wrong approach. I would create an executable and set it up as a scheduled task to run every 24 hours. Not sure what OS you are using, but assuming it's windows its fairly easy http://windows.microsoft.com/en-au/windows7/schedule-a-task


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