HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Class for easier to read IProgress<Class> handling

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
handlingreadiprogressforclasseasier

Problem

The recommended way to report something as "progress" back from a async function is to use IProgress or IProgress.

When the caller invokes the async function, it can provide an implementation for IProgress. This object is then used inside the called instance to report back progress.

For a better description of IProgress, please see Reporting Progress from Async Tasks by Stephen Cleary.

Example code:

Code for the consumer (that wants to receive progress):

//Create progress object
Progress progressRunner = new Progress();
progressRunner.ProgressChanged += RunnerProgressUpdate;

//Pass it to the async function
bool result = await simpleRunner.RunAsync(progressRunner);


Called function (that reports progress back):

public async Task RunAsync(IProgress Progress)
{
  if (Progress != null)
  {
     ProgressDetail rp = new ProgressDetail();
     rp.Starting = true;
     Progress.Report(rp);
  }
}


Description:

Although the implementation inside the called function is trivial, IMHO it creates too much code for the simple task of reporting back progress. The if check is required because it is perfectly legal that a caller passed in null if no progress is desired.

Beside this, it is also important that the reported object is used only once. Because the object might be handled in an entirely different thread, each call of Report() must yield a new instance.

Quote from Reporting Progress from Async Tasks by Stephen Cleary:


...it means you can’t modify the progress object after it’s passed to Report. It is an error to keep a single “current progress” object, update it, and repeatedly pass it to Report.

Goal:

The goal for this class is:

-
Get rid of the NULL check altogether in order to make the code easier to read (I simply like code that can be read from top to down without conditions whenever possible).

-
Prevent the reuse of the object that is reported as progress and throw an exception if someone tries it anyway (Because this

Solution

public class ProgressReporter


According to Framework Design Guidelines about naming of types, the name of a type parameter should start with a T, here, that would be TReportedObject.

Also, I think that specifying that it's an object is redundant, so something like TReported should be enough.

public ReportedObject Content { get; private set; }


I don't like that the setter is private. Since you usually need to specify multiple properties at the same time, I would prefer to use object initializer:

new ProgressDetail { Starting = true }


But making the setter private means that this is not possible.

public ProgressReporter(IProgress IProgress, bool CreateNewInstanceAfterReport)


Again, based on Framework Design Guidelines, this time about naming parameters, parameter names should use camelCase, not PascalCase. Here that would be progress (I don't think iProgress makes sense) and createNewInstanceAfterReport.

_rearmAfterReport = CreateNewInstanceAfterReport;


Why does the field have a different name than the parameter? That's confusing.

Also, if you delegated from the simpler constructor to the more complicated one, it would make your code simpler. Or just have a single constructor overload with an optional parameter instead.

Code Snippets

public class ProgressReporter<ReportedObject>
public ReportedObject Content { get; private set; }
new ProgressDetail { Starting = true }
public ProgressReporter(IProgress<ReportedObject> IProgress, bool CreateNewInstanceAfterReport)
_rearmAfterReport = CreateNewInstanceAfterReport;

Context

StackExchange Code Review Q#77819, answer score: 4

Revisions (0)

No revisions yet.