Can this cycle be written more succinctly?

I have two queues , let them say A and B, on which I execute the following algorithm:

while (queueA.Count > 0) { var elemA = queueA.Peek(); var elemB = queueB.Peek(); if (AreSimilar(elemA, elemB)) { Debug.Assert(elemA.SomeProperty == elemB.SomeProperty); queueA.Dequeue(); queueB.Dequeue(); } else { break; } } 

Something tells me that this can be written more succinctly; Peek () and Dequeue () can be combined in one operation, since Dequeue () returns the same element as Peek (), and the if statement can be connected to the while statement, avoiding an explicit break. I just don’t see how to preserve exactly the same behavior, i.e. I do not want to delete the item if it does not satisfy the condition in the "if".

+4
source share
3 answers

You can try this code, which moves the assignment into an AreSimilar call:

 QueueElement elemA, elemB while (queueA.Count > 0 && AreSimilar(elemA = queueA.Peek(), elemB = queueB.Peek())) { Debug.Assert(elemA.SomeProperty == elemB.SomeProperty); queueA.Dequeue(); queueB.Dequeue(); } 

Please note that this is not necessarily more readable. In fact, your version is pretty good in terms of readability. The only thing I would like to do is to invert the condition to reduce nesting, but I would leave everything else:

 while (queueA.Count > 0) { var elemA = queueA.Peek(); var elemB = queueB.Peek(); if (!AreSimilar(elemA, elemB)) { break; } Debug.Assert(elemA.SomeProperty == elemB.SomeProperty); queueA.Dequeue(); queueB.Dequeue(); } 
+4
source

What is the likelihood that A and B will be similar? If the likelihood is high, you can always run the w / supposed common script and pop them (or Dequeue) their ASSUMING, they will be similar, and then just worry about bringing them back if they are not similar ...

+2
source

You can simplify the loop with the method, but you can think about it:

 static bool DequeuePairIf<T>( Func<T, T, bool> predicate, Queue<T> queueA, Queue<T> queueB) { if (queueA.Count != 0 && queueB.Count != 0 && predicate(queueA.Peek(), queueB.Peek()) ) { queueA.Dequeue(); queueB.Dequeue(); return true; } return false; } 

Then your loop will look like this:

 while (DequeuePairIf(AreSimilar, queueA, queueB)) { } 

But I doubt whether this refactoring of readability will help or harm. Firstly, it is significantly more than the source code. On the other hand, smaller code is not always readable.

(I removed assert to simplify the logic here. If you still need to assert, you will have to save the results of Peek() calls, as in the source code.)

+1
source

All Articles