combining two do while loops into one or any better refactoring possible

Question

I have an HTML video player that runs random videos. I have the following function that tests if the video is marked completed based on the video seen percentage. VideoCompletionPercentage is a global variable that is set by reading it from the database. I have to test for two conditions as follows:

verify video is marked complete if it is seen till a VideoCompletionPercentage set from the DB.
verify video is not marked complete if it is skipped forward and seen till 100%

I am switching between two while loops based on a skip bool variable. Is there any way to combine these two do while loops into one by manipulating the while condition or any other way? Thanks in advance for your inputs.

private void VideoPecentageCompletion(bool skip)
        {
            double videoSeenPercentage;
            if (skip)
            {
                do
                {
                    double.TryParse(Driver.FindElement(By.CssSelector("div[class=played]")).GetAttribute("aria-valuemax"), out double totalVideo);
                    double.TryParse(Driver.FindElement(By.CssSelector("div[class=played]")).GetAttribute("aria-valuenow"), out double videoProgress);
                    videoSeenPercentage = Math.Floor(videoProgress / totalVideo * 100);
                } while (videoSeenPercentage < VideoCompletionPercentage);
            }
            else
            {
                do
                {
                    double.TryParse(Driver.FindElement(By.CssSelector("div[class=played]")).GetAttribute("aria-valuemax"), out double totalVideo);
                    double.TryParse(Driver.FindElement(By.CssSelector("div[class=played]")).GetAttribute("aria-valuenow"), out double videoProgress);
                    videoSeenPercentage = Math.Floor(videoProgress / totalVideo * 100);
                } while (videoSeenPercentage < 100);
            }
        }

You can at least refactor the `double.TryParse(...)` lines – Rui Jarimba Oct 04 '18 at 09:52 — Rui Jarimba, Oct 04 '18 at 09:52

score 1 · Answer 1 · answered Oct 04 '18 at 09:52

You can use && and || to combine the conditions and as a result use just one loop:

do
{
    double.TryParse(Driver.FindElement(By.CssSelector("div[class=played]")).GetAttribute("aria-valuemax"), out double totalVideo);
    double.TryParse(Driver.FindElement(By.CssSelector("div[class=played]")).GetAttribute("aria-valuenow"), out double videoProgress);
    videoSeenPercentage = Math.Floor(videoProgress / totalVideo * 100);
} while ((skip && videoSeenPercentage < VideoCompletionPercentage) || (!skip && videoSeenPercentage < 100));

score 1 · Accepted Answer · answered Oct 04 '18 at 09:58

1

you could just use operateur "?"

do
 {
  :
  :
 }while (videoSeenPercentage < (skip ? VideoCompletionPercentage : 100));

answered Oct 04 '18 at 09:58

Frenchy

16,386
3
16
39

I should have thought about it myself :-) – Zohar Peled Oct 04 '18 at 10:28
I was looking for this stroke of genius thanks Frenchy – melleck Oct 04 '18 at 10:35

score 0 · Answer 3 · answered Oct 04 '18 at 10:20

If the div is staticaly you can introduce it to a local variable and avoiding recalculation. Using the @Zohar Peled's answer it might look as that:

private void VideoPecentageCompletion(bool skip)
{       
    var playedDiv = Driver.FindElement(By.CssSelector("div[class=played]");
    do
    {
        double.TryParse(playedDiv.GetAttribute("aria-valuemax"), out double totalVideo);
        double.TryParse(playedDiv.GetAttribute("aria-valuenow"), out double videoProgress);
        videoSeenPercentage = Math.Floor(videoProgress / totalVideo * 100);
    } 
    while ((skip && videoSeenPercentage < VideoCompletionPercentage) || (!skip && videoSeenPercentage < 100));  
}

Thanks Roni for the local variable suggestion, I will combine it with Frenchy's code above :) — melleck, Oct 04 '18 at 10:36

score 0 · Answer 4 · answered Oct 04 '18 at 10:42

private void VideoPecentageCompletion(bool skip)
{
    double videoSeenPercentage;
    double percentage;

    do
    {
        TryGetAttributeValue("aria-valuemax", out double totalVideo);
        TryGetAttributeValue("aria-valuenow", out double videoProgress);
        videoSeenPercentage = Math.Floor(videoProgress / totalVideo * 100);

        percentage = skip ? VideoCompletionPercentage : 100;
    } while (videoSeenPercentage < percentage);
}

private bool TryGetAttributeValue(string attributeName, out double value)
{
    string attributeValue = Driver.FindElement(By.CssSelector("div[class=played]")).GetAttribute(attributeName);
    return double.TryParse(attributeValue, out value);
}

score 0 · Answer 5 · answered Oct 05 '18 at 06:47

Thanks all for your feedback and suggestions with special thanks to Roni and Frenchy.

My final optimized, refactored and tested code is as follows:

 private void VideoPecentageCompletion(bool skipForward)
    {
        double videoSeenPercentage;
        var playedDiv = Driver.FindElement(By.CssSelector("div[class=played]"));

        do
        {
            double.TryParse(playedDiv.GetAttribute("aria-valuemax"), out double totalVideo);
            double.TryParse(playedDiv.GetAttribute("aria-valuenow"), out double videoProgress);
            videoSeenPercentage = skipForward ? Math.Ceiling(videoProgress / totalVideo * 100) : Math.Floor(videoProgress / totalVideo * 100);
        } while (videoSeenPercentage < (skipForward ? 100 : VideoCompletionPercentage));
    }

combining two do while loops into one or any better refactoring possible

5 Answers5