Popup message queueing

Question

I have a WPF application where I have a function for generating an overlaying info grid. PopupBox

I have written some code for queueing the messages(IpopupMessage) so that each displays a certain amount of time

public interface IPopupMessage
{
 string Description { get; set; }
 TimeSpan Duration { get; set; }
 SolidColorBrush MessageColor { get; set; }
 string Title { get; set; }
}

This is the code for displaying and hiding the UI-element

private readonly Queue<IPopupMessage> PopupMessageQueue = new Queue<IPopupMessage>();
public void DisplayPopupmessage(IPopupMessage obj)
{
 this.PopupMessage = obj;
 if (obj.Duration > TimeSpan.Zero) this.HideErrorMessageAfter(obj.Duration);
}
protected async void HideErrorMessageAfter(TimeSpan delayTime)
{
 await Task.Delay(delayTime);
 if (this.PopupMessageIsVisible) this.PopupMessageIsVisible = false;
}
private void CancelPopupMessage()
{
 this.PopupMessageIsVisible = false;
}
private async void UpdatePopupMessage(IPopupMessage obj)
{
 //Adding the message to the queue
 this.PopupMessageQueue.Enqueue(obj);
 //never ending loop
 while (true)
 //PopupMessageIsVisible is a boolean with binding to the UI element
 if ((this.PopupMessageQueue.Count > 0) && (this.PopupMessageIsVisible == false))
 {
 this.PopupMessageIsVisible = true;
 this.DisplayPopupmessage(this.PopupMessageQueue.Dequeue());
 }
 else
 {
 await Task.Delay(TimeSpan.FromSeconds(1));
 }
}

Is there a cleaner more efficient way for this? I have tried some variants with single thread TPL actionblock, but the timer in HideErrorMessageAfter would set PopupMessageIsVisible false and cut the next message short.

I also tried creating a continuous Queue where every queued item would get consumed automatically, but I could not get it to work.

Do you have any tips for making this code more efficient?

• forsvarir: You need to include the code in the actual question, not just link to it.
• Sondre: OK, I will edit it. Thanks for the heads up :)
• allquixotic: Before posting your actual code, you might want to read through all answers here, not only the one about having to post your code in the question. It looks like your app might be incomplete and you're looking to flesh out the concept, which this isn't really the place for.

Answer 1

I think instead of looping indefinitely over the queue you should only start it when the first popup is queued via the DisplayPopupmessage and loop only as long as there are any popups. After you've shown everything you should exit the loop and start agian when something arrives.

---

For this job you should have a dedicated service that knows how to handle it. Currently it looks like the logic is within the popup-message.

I used these models for testing:

interface IPopupMessage
{
 string Message { get; }
 TimeSpan Timeout { get; }
}
class PopupMessage : IPopupMessage
{
 public string Message { get; set; }
 public TimeSpan Timeout { get; set; }
}

A simple service could be implemented like this one to handle the queue:

class PopupMessageService
{
 private bool _started;
 private readonly Queue<IPopupMessage> _popupMessageQueue = new Queue<IPopupMessage>();
 public void DisplayPopupMessage(IPopupMessage popup)
 {
 Console.WriteLine($"Equeued \"{popup.Message}\".");
 _popupMessageQueue.Enqueue(popup);
 Start();
 }
 private async void Start()
 {
 if (_started) { return; }
 _started = true;
 Console.WriteLine("Service started.");
 while (_popupMessageQueue.Any())
 {
 await Task.Run(async () =>
 {
 var msg = _popupMessageQueue.Dequeue();
 Console.WriteLine($"Showing \"{msg.Message}\".");
 await Task.Delay(msg.Timeout);
 });
 }
 _started = false;
 Console.WriteLine("Service stopped.");
 }
}

The real message should have a method like Show(CacellationToken token) that you use to display the message and to hide it via the token.IsCancellationRequested and token.ThrowIfCancellationRequested(). You should handle the OperationCanceledException in order to proceed to the next popup.

It's just for demonstration purposes so I left the Console.WriteLine there.

And here's one more example how to use the above service:

void Main()
{
 Messages();
}
static async void Messages()
{
 var popupMessageService = new PopupMessageService();
 var messages = new IPopupMessage[]
 {
 new PopupMessage { Message = "Foo", Timeout = TimeSpan.FromSeconds(3) },
 new PopupMessage { Message = "Baz", Timeout = TimeSpan.FromSeconds(5) },
 new PopupMessage { Message = "Qux", Timeout = TimeSpan.FromSeconds(1) },
 new PopupMessage { Message = "Bar", Timeout = TimeSpan.FromSeconds(2) },
 };
 foreach (var msg in messages)
 {
 popupMessageService.DisplayPopupMessage(msg);
 // For test purposes assuming a message arrives every 3 seconds.
 await Task.Delay(TimeSpan.FromSeconds(3)); 
 }
}

The output is:

Equeued "Foo".
Service started.
Showing "Foo".
Service stopped.
Equeued "Baz".
Service started.
Showing "Baz".
Equeued "Qux".
Showing "Qux".
Service stopped.
Equeued "Bar".
Service started.
Showing "Bar".
Service stopped.

• Heslacher: Why don't you use the Count property instead of Any()in the while condition ?
• t3chb0t: @Heslacher because I like it more ;-) To me it reads more naturally while(x.Any()) then while(x.Count > 0). I don't have to think about the first one whereas I have to stop for a second and think about the magic 0 in the >0. Call it personal preference.
• Heslacher: Ok, just keep in mind that each time you call Any() a call to GetEnumerator() of the source is made.
• t3chb0t: @Heslacher sure, however I don't think it's usually a bad thing. I mean these extensions are there for convienience so I guess we should use them unless you need to optimize the last bit of the algorithm but then you'd probably run it in a profiler to identify the bottlenecks ;-)