C#•2y ago

❔ Calculating estimated time is inaccurate

Hello, I try to calculate the remaining time. The problem is that the estimated remaining time is pretty inaccurate. The class says that each job takes around 7 seconds, but it is around ~1 second. What might the problem here? Is it because I do use seconds over milliseconds?

public class WorkerTime
{
    /// <summary>
    /// Total amount of jobs
    /// </summary>
    public int Total { get; }

    /// <summary>
    /// Amount of jobs done
    /// </summary>
    public int Done { get; private set; }

    /// <summary>
    /// Estimated remaining time
    /// </summary>
    public TimeSpan TimeRemaining { get; private set; } = TimeSpan.Zero;


    private DateTime Start { get; } = DateTime.UtcNow;

    public WorkerTime(int jobs)
    {
        Total = jobs;
    }

    public void Execute()
    {
        Done++;

        TimeSpan timeTaken = DateTime.UtcNow - Start;
        int jobsLeft = Total - Done;

        int totalSeconds = (int)timeTaken.TotalSeconds;

        if (totalSeconds == 0) 
            return;

        int timeRemaining = Done / totalSeconds * jobsLeft;
        TimeRemaining = TimeSpan.FromSeconds(timeRemaining);
    }

    public TimeSpan Stop()
    {
        return DateTime.UtcNow - Start;
    }
}

public class WorkerTime
{
    /// <summary>
    /// Total amount of jobs
    /// </summary>
    public int Total { get; }

    /// <summary>
    /// Amount of jobs done
    /// </summary>
    public int Done { get; private set; }

    /// <summary>
    /// Estimated remaining time
    /// </summary>
    public TimeSpan TimeRemaining { get; private set; } = TimeSpan.Zero;


    private DateTime Start { get; } = DateTime.UtcNow;

    public WorkerTime(int jobs)
    {
        Total = jobs;
    }

    public void Execute()
    {
        Done++;

        TimeSpan timeTaken = DateTime.UtcNow - Start;
        int jobsLeft = Total - Done;

        int totalSeconds = (int)timeTaken.TotalSeconds;

        if (totalSeconds == 0) 
            return;

        int timeRemaining = Done / totalSeconds * jobsLeft;
        TimeRemaining = TimeSpan.FromSeconds(timeRemaining);
    }

    public TimeSpan Stop()
    {
        return DateTime.UtcNow - Start;
    }
}

5 Replies

mtreit•2y ago

First off, use Stopwatch for things like timeTaken Instead of DateTime subtraction And yes I wouldn't use seconds, that's not very granular

HalfbaxOP•2y ago

Thanks. I will change that. The new class works great but the code smells imo. Any suggestions for improvement?

public class WorkerTime
{
    /// <summary>
    /// Total amount of jobs
    /// </summary>
    public int Total { get; }

    /// <summary>
    /// Amount of jobs done
    /// </summary>
    public int Done { get; private set; }

    /// <summary>
    /// Estimated remaining time
    /// </summary>
    public TimeSpan TimeRemaining { get; private set; } = TimeSpan.Zero;

    private readonly Stopwatch _totalWatch = new();
    private readonly Stopwatch _taskWatch = new();

    public WorkerTime(int jobs)
    {
        Total = jobs;
        _totalWatch.Start();
        _taskWatch.Start();
    }

    public void CalculateRemainingTime()
    {
        Done++;
        
        long timeRemaining = _taskWatch.ElapsedMilliseconds * (Total - Done);
        TimeRemaining = TimeSpan.FromMilliseconds(timeRemaining);

        _taskWatch.Restart();
    }

    public TimeSpan Stop()
    {
        _totalWatch.Stop();
        _taskWatch.Stop();

        return _totalWatch.Elapsed;
    }
}

public class WorkerTime
{
    /// <summary>
    /// Total amount of jobs
    /// </summary>
    public int Total { get; }

    /// <summary>
    /// Amount of jobs done
    /// </summary>
    public int Done { get; private set; }

    /// <summary>
    /// Estimated remaining time
    /// </summary>
    public TimeSpan TimeRemaining { get; private set; } = TimeSpan.Zero;

    private readonly Stopwatch _totalWatch = new();
    private readonly Stopwatch _taskWatch = new();

    public WorkerTime(int jobs)
    {
        Total = jobs;
        _totalWatch.Start();
        _taskWatch.Start();
    }

    public void CalculateRemainingTime()
    {
        Done++;
        
        long timeRemaining = _taskWatch.ElapsedMilliseconds * (Total - Done);
        TimeRemaining = TimeSpan.FromMilliseconds(timeRemaining);

        _taskWatch.Restart();
    }

    public TimeSpan Stop()
    {
        _totalWatch.Stop();
        _taskWatch.Stop();

        return _totalWatch.Elapsed;
    }
}

Monsieur Wholesome•2y ago

public class WorkerTime
{
    /// <summary>
    /// Total amount of jobs
    /// </summary>
    public int Total { get; }

    /// <summary>
    /// Amount of jobs done
    /// </summary>
    public int Done { get; private set; }

    /// <summary>
    /// Estimated remaining time
    /// </summary>
    public TimeSpan TimeRemaining { get; private set; } = TimeSpan.Zero;

    private long _start;

    public WorkerTime(int jobs)
    {
        Total = jobs;
        _start = Stopwatch.GetTimestamp();
    }

    public void Execute()
    {
        Done++;
        
        var timeTaken = Stopwatch.GetElapsedTime(_start);
        TimeRemaining = (Total - Done) * timeTaken;
    }

    public TimeSpan Stop()
    {
        return Stopwatch.GetElapsedTime(_start);
    }
}

public class WorkerTime
{
    /// <summary>
    /// Total amount of jobs
    /// </summary>
    public int Total { get; }

    /// <summary>
    /// Amount of jobs done
    /// </summary>
    public int Done { get; private set; }

    /// <summary>
    /// Estimated remaining time
    /// </summary>
    public TimeSpan TimeRemaining { get; private set; } = TimeSpan.Zero;

    private long _start;

    public WorkerTime(int jobs)
    {
        Total = jobs;
        _start = Stopwatch.GetTimestamp();
    }

    public void Execute()
    {
        Done++;
        
        var timeTaken = Stopwatch.GetElapsedTime(_start);
        TimeRemaining = (Total - Done) * timeTaken;
    }

    public TimeSpan Stop()
    {
        return Stopwatch.GetElapsedTime(_start);
    }
}

Does this fulfill your needs?

mtreit•2y ago

That seems like a convoluted way to use Stopwatchk, what's the advantage of doing that instead of just using the Elapsed property?

Accord•2y ago

Was this issue resolved? If so, run /close - otherwise I will mark this as stale and this post will be archived until there is new activity.

Gaming

Programming

❔ Calculating estimated time is inaccurate