# Tag Info

14

Interesting idea and well-done! Naming I really don't like the names. The names like clsMultiThread is somewhat misleading, since as you noted they don't actually provide any true multi-threading. A unwary user would expect it to work with anything and would be disappointed when all of their queued works painfully completes synchronously. ;) Also, we are ...

13

I like it, you could clean up the commented out code and possibly replace $window.googleMapsInitialized = function () { mapsDefer.resolve(); }; with$window.googleMapsInitialized = mapsDefer.resolve; Personally, I would re-arrange the code a tiny bit, keep the URL completely on top, keep asyncLoad and the call closer together. Keep ...

13

This is an ugly old pattern. Why don't just try the newer async/await? You already use Task in your tests anyway. In order to implement it the awaitable way you just need to use a different API, in this case AcceptTcpClientAsync and build everything on top of it. With the CancellationToken you can now better control the server. public class TcpServer : ...

12

A better alternative to the busy wait loop would be to use a Semaphore - in this case a SemaphoreSlim would probably be the most appropriate. Semaphores are not reentrant. This would simplify the implementation to: struct DumbAsyncMutex { private SemaphoreSlim _Semaphore = new SemaphoreSlim(1, 1); public async Task WithMutexAsync(Func<Task> t)...

11

Rather than using System Events to generate keystrokes, consider using TextEdit itself to insert text. tell application "TextEdit" activate tell first document to set its text to its text & "Hello World!\n" end tell There is a difference, though: this version always appends "Hello World!" to the end of the document, rather than wherever the ...

11

Sorry to disappoint, but you did everything right! This is a quite good example of using RxUI, Akavache, and Refit together. The only thing I would change, is to not immediately call LoadItems.ExecuteAsyncTask in the ViewModel constructor. Invoking this in the VM constructor means that your VM class becomes more difficult to test, because you always have ...

10

10

There are a few items to make this code better. Naming. Once is an OK name for the class, but the method name IsDone is a problem. This is an 'atomic' operation that sets values, as well as gets values. A method called something like "Trigger", and changing the class name to a common term like OneShot, will give you the semantics like: private readonly ...

10

If it's really just removing an item from a dictionary (and not e.g. doing synchronous file IO), then Alternative 1 is the best. When you're using async for scalability (e.g. in ASP.NET), using Task.Run() like this won't help you (since the number of threads used stays the same), it will only hurt you a bit (since Task.Run() has some overhead). When you're ...

10

Reflection is slow. That line probably doesn't help your performance : var model = (T)Activator.CreateInstance(typeof(T)); I notice that you always create your instance with parameterless constructors. So you could add the type constraint new() in your class. That means you could do : var model = new T(); That'd be faster and well... clearer. You also ...

10

Quick Review An API like this, dealing with thread-sensitive operations, requires time and effort to test and review rigorously. When I will find this time, I will do a thorough review. But here are some things I notice right off the bat. CommMemoryDisposedException should inherit from ObjectDisposedException. This way, consumers can handle your exception ...

8

Have you considered using HttpClient class from .Net 4.5+? That makes your code lot cleaner. Also why not take Async pattern all the way? From your initiator (button click or main etc) you can call a async method, which will call all the downstream methods. That way it will be more scalable (as framework will take care of worker threads, scheduling etc)and ...

7

I am blocking the call using AsTask().Result, is this the most appropriate method? Can't you use await? Most testing frameworks (apparently including MSTest) support async tests. That way, you could work directly with the inner exception instead of the AggregateException. if (ae.InnerException.GetType() == typeof(InvalidOperationException)) { Assert....

7

Assuming that you want to stick with the original plan to insert text wherever the cursor happens to be, you would need something more deterministic than an arbitrary delay. This script covers all the scenarios that I can think of: tell application "System Events" -- In case TextEdit was already running and all windows were closed repeat ...

7

First, I want to point out the behavior of method that is declared as async void. When the code reaches this line: var result = await data; The code following after _service.GetAnimateur is executed. async void is there only for event handlers. So using this signature creates more problems than it solves. Second, I may not see the whole picture, but your ...

7

I know this is technically outside the scope of the review here, but your first parameter's type is Method. This enum desperately needs to be renamed. When I see Method, I cannot help but think of a programming method... I think you're passing a pointer to a class's method here. This type needs a better, more descriptive name. Our sender parameter doesn'...

7

It's hard to say how to write the ideal method for your use-case because you don't specify some of the details that would be necessary to know, such as: How often will your code be making this query? Is the connection always to the same IMAP server, username and password? (if it'll be called fairly frequently with the same server/username/password combo, ...

7

No that isn't right. If your query throws an error you won't be closing the connection. Swallowing exceptions like that (even though you're logging it) is generally bad as well. How does the user know anything has gone wrong? If you can, just use async and await with a using: public async Task DeleteDataAsync(...) { using (var connection = ...

7

7

General Your model should not return an ObservableCollection because that collection should be used only if its change notification ability is needed (e.g. with data binding). The method GetDocuments should return an Array or an IEnumerable. The method InitializeColumns also seems to be GUI related. Consider moving it to the view model. Background ...

7

From the SendAsync docs: After calling SendAsync, you must wait for the e-mail transmission to complete before attempting to send another e-mail message using Send or SendAsync. So no, this may not work properly. You need to register an event handler on the SmtpClient.SendCompleted event so that you know the message has sent successfully. I'm going ...

7

This is way above my expertise, but maybe adding an answer would cause more views/answers? Also, what's that beginner tag doing there? ;) I want to say first off, really solid work. That's probably why there hasn't been too much activity here. Maybe some of this will seem like nit-picks and if so, Sorry! ByRef or ByVal arguments Every one of the arguments, ...

7

Ugly typedef I'm not a big fan of this: typedef std::future<int> FutureResultInt; It's not significantly shorter or easier to read, it doesn't isolate the user from an underlying type, and it only serves to slow down my reading every time I hit it. That's somewhat subjective, of course, but I don't believe it adds value. I'd be happier with it if ...

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_; ...

7

EnqueueAction may want to throw an ObjectDisposedException if the queue is disposed, depending on the precise API you want. I don't see the value of IsBusy, and it won't be cleared if the task is cancelled. I little padding inside the while loop would make that code much easier to understand. I'd be temped to pull the logic for retrieving the next action ...

6

Well, you start two Tasks each reading and writing a file but not the same. So basically it should be ok. Some remarks: Task.Factory.StartNew returns a Task. You probably want to wait until the tasks you have started are finished before you quit from main which you can do with Task.WaitAll. Something along these lines: var task1 = Task.Factory.StartNew(() =...

6

It does show very good example for the progressbar in WinForms. Here are my code-review outputs for you. Closebutton_Click calls the other click "handlers". This is not the common way. Implement another method and call it from those handlers. I couldnt understand the logic why are you asking the cancellation before and after invoking the DoSomething(). ...

6

I'm not all that familiar with async/await (I really need to get going on this), but I find your code pretty clear, except why the magic number 5 is being used for an initialCount. I like your usage of var, but the comments are a little redundant, they say nothing much that the code doesn't already tell. It would have been nice to see the actual code in ...

6

SemaphoreSlim implements IDisposable and should be disposed of, preferably using the using statement.

6

The fact that the lambda you're passing to Task.Run is async, along with the comment, implies that the work being done is not CPU bound work, but rather is IO bound work. Given this, there is no need to call Task.Run here. Task.Run allows you to turn synchronous CPU bound work into asynchronous CPU bound work. However, the work that you have is neither ...

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