melleck
melleck

Reputation: 316

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

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:

  1. verify video is marked complete if it is seen till a VideoCompletionPercentage set from the DB.
  2. 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);
            }
        }

Upvotes: 0

Views: 83

Answers (5)

melleck
melleck

Reputation: 316

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));
    }

Upvotes: 0

Rui Jarimba
Rui Jarimba

Reputation: 18084

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);
}

Upvotes: 0

Roni
Roni

Reputation: 403

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));  
}

Upvotes: 0

Frenchy
Frenchy

Reputation: 17027

you could just use operateur "?"

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

Upvotes: 1

Zohar Peled
Zohar Peled

Reputation: 82504

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));

Upvotes: 1

Related Questions