views:

424

answers:

13

I've struggled for the last couple of months to come up with some clean code to report progress to a user. Everything always seems to boil down to:

ReportProgress("Starting Task 1");
doTask1();
ReportProgress("Task 1 is done");

ReportProgress("Starting Task 2");
doTask2();
ReportProgress("Task 2 is done");

//etc... where report progress does some form of output to the user.

The good coder in me screams "There's got to be a cleaner way!" But I'm stumped. Any thoughts?

EDIT :: I'm looking more for information on architectural information as opposed to implementation specific. The code given is very oversimplified.

+3  A: 

Could you perhaps use Aspect Oriented Programming, and devise a Progress Aspect?

There are a number of AOP implementations. In the Java world, the 2 most common are AspectJ and Spring (which uses either AspectJ, or Proxy based aspects).

toolkit
lol - yes by all means learn a whole new paradigm to solve a simple problem!
Steven A. Lowe
I guess it depends on the amount of code. For a small bit of code, then sure, an in-code progress approach will always be the simplest. But as the amount of code increases, you will end up having more code for progress reporting, obfuscating the core logic itself. Just my opinion.
toolkit
@Steven A. Lowe: I wonder if people responded in much the same way to the recommendation of using objects when object oriented programming was in its infancy.
Waylon Flinn
@[Waylon Flinn]: the response I usually got was "what's an object?" and "I don't understand?" ;-) But seriously, AOP is hardly necessary for this task.
Steven A. Lowe
+1  A: 

It would be natural to have the reporting inside the doTask() calls.
Typically the reporter would be a singleton that all the objects sent messages to and a reporter class was reponsible for deciding if and where to show it - statusbar, log file, stderr etc.

Martin Beckett
This couples the methods to the reporter class, which means you can't call the method without reporting (or adding a parameter to control whether or not you call the reporter). That may be OK - just sayin'.
MusiGenesis
YOu normally let the reporter object decide if it is going to report. Unless you are concerned about the overhead of reporting essages which aren't going to be used.
Martin Beckett
+1  A: 

Assuming the pattern in what you're doing is:

  • log task start
  • do task
  • log task end

you can have a "Task" class (parent for all your tasks), whose do() method is subclassed and automatically logs start and end of task.

Just an idea.

friol
A: 

You could call ReportProgress from inside the doTask methods, that might make it look a little cleaner, instead you would just have:

doTask1();
doTask2();

The reporting would be handled inside those methods.

You could use AOP, but my brain screams KISS!!(Keep It Simple Stupid) in this case. If this is just a simple representation of something more complicated that you are dealing with, AOP could be an option.

Ryan Thames
+3  A: 

You could create a Task class with a name property and delegate. Put each task in a collection, then iterate through it, printing the message and invoking the delegate for each task.

Jon B
+1  A: 

I wouldn't hand-code the numeric parts of the displayed messages like that (any time you need to add or remove actions or change the sequence you've got a mess of cut-and-paste to do). You'd want whatever object is handling the ReportProgress method to auto-increment itself as it goes along.

MusiGenesis
+8  A: 

set up your tasks as an event stream, and have the event-processing 'engine' report progress. Each event instance can have its own name, progress-reporting blurb/template, etc. if you want to go that far

if this is a pattern that occurs often, it is worth the effort for the infrastructure. When you're done, the usable code might look something like:

EventQueue tasks = new EventQueue();
tasks.Add(new TaskEvent(this.doTask1,"Foo-ing the bar"));
tasks.Add(new TaskEvent(this.doTask2,"Bar-ing the foo"));
tasks.Add(new TaskEvent(this.doTask3,"Glitching and whinging"));
...
tasks.Execute(this.ProgressEventHandler);
Steven A. Lowe
Wow, a minty code smell. :) +1
Allain Lalonde
@[Allain Lalonde]: thanks, it's so fresh you can still smell the compiler!
Steven A. Lowe
+1  A: 

A fairly simple and clean way would be to create an abstract class that has a do() method and abstract doTask() and getName() methods:

do() {
    ReportProgress("Starting " + this.getName());
    doTask();
    ReportProgress("Finished " + this.getName());
}

Then in your tasks:

class Task1 extends Task {
    getName(){return "Task 1";}
    doTask() {
        //do stuff here
    }
}

You could then have a Task that has a doTask() method that runs do() on a whole bunch of other tasks. This could easily be recursive, in that any Task might then run a number of sub-Tasks.

Adam Jaskiewicz
+1  A: 

+1 on the AOP suggestion. This is a classic crosscutting concern that AOP would solve elegantly.

A: 

Unfortunately I think the best way to do this does depend on the details -- at the very least what language you are using. For example in python you could use a context manager to allow for writing code like this:

with progress_report("Task 1"):
    do_task_1()

This could, e.g., ensure that the "Task 1 is done" is reported even if do_task_1() raises an exception. If you wanted to, you could handle exceptions separately and print something different like "Task 1 failed" or "Task 1 aborted."

John
A: 

In our tool kit, we have a task controller that manages tasks. A task is run as a thread. In addition to typical thread support, a task supports progress methods. One possible view onto the progress is a visual progress bar with a title that refers to task name and step within task. To support visible statistics and status, the code must make occasional calls to the task's progress method. Typically, this is done within for loops since the percentage progress can be estimated by the current index divided by the limit.

The task controller is a useful place to add global thread control, status probes, other statistics and performance measurement hooks. Some multithreaded bugs and timing issues can be analyzed by examining the controller's state, and the state of all the tasks.

dongilmore
+2  A: 

It depends how much config is needed on the fly, I would expect the code to be very general, and have the tasks be setup via spring or any ioc container.

This would all be in a spring config: The xml config would supply the task object with its name and parameters. then add these tasks to a collection, and hand that collection to the taskrunner.

The task runner is then code that signals the stop and start of each task, but each task then is free to give specific status of how it is going. Also the taskrunner will catch any exceptions and keep going if something errs out. It could be made configurable to say that certain tasks depend on the completion of others, and some tasks should halt everything if they fail.

I disagree that AOP should be used here. overkill.

A: 

If you are using .NET, I would suggest using the Policy Injection Application Block from Enterprise Library (Minimum Version: 3.1). I used a similar stuff to do a "Revert to the application pool identity" for a website that was heavy on impersonation.

You could do the same with your task. You could simply define a task class with a factory that build the object with the Enterprise Library object factory and automatically add a "Before" and "After" message to the task. That would give EXACTLY what you want with the elegence that is required.

Have fun!

Maxim