2

Normally, I would solve this via something like the following:

task = _taskProvider.getNextFromQueue();

if (task.id == TaskExecutor.TasksEnum.One) {
    _taskExecutor.executeTaskOne();
    return;
} else if (task.id == TaskExecutor.TasksEnum.Two) {
    _taskExecutor.executeTaskTwo();
    return;
} else if (task.id == TaskExecutor.TasksEnum.Three) {
    _taskExecutor.executeTaskThree();
    return;
}
//and so on...

I could switch if-else to switch if needed. I do believe there exists a better way to implement similar code. One thing that comes to mind is to use a table (map) to store task ids and pointers to corresponding functions, but I am not sure if it's useful and provide adequate performance.

Another thing that concerns me is notification mechanism. This problem occurs when TaskProvider and TaskExecutor are two separate entities that operate on different threads. It becomes even worse if several actions take some time to finish execution and I need to create a logic to make a caller wait for the result.

For example, if executeTaskTwo() takes some time to complete, I would execute it in a background thread, meaning executeTaskTwo() would return nearly immediately, while the task itself would still be executing. But how do I notify the caller about it's completion once it's over?

I guess that the solution involves heavy modification, maybe some event libraries (I've never worked with these before). I need a starting point. I guess there exist a patterns of some sort, but I do not know which one exactly.

Update 1:

Some more info about general architecture and problems There are three entities:

  • Hardware - manages hardware signals (inputs, outputs). Cares only about hardware, does not ask questions about why?
  • Network - manages remote connections to the device. Cares only about data transmission. Again, it does not care about what the data represents.
  • Controller - controls the actual device's task/algorithm (what this device does). This is the brain of the device.

Each entity operates on it's own thread. Sometimes, an event happen and one entity needs to send this event to the other entity (or all of them). For example, a command from network turns on some LED that is managed by hardware, or, a failure to turn this LED should be signaled to both network (notify remote clients) and controller (execute emergency stop).

Right now each entity (class) has a set of function (methods) that other entities call when they this entity to do something. If a called function takes time to execute, a caller entity is blocked, which is bad. Spawning background thread to handle each command (or event) also feels wrong. Right now each entity has it's own queue of commands and executes it's commands sequentially (execute(queue.getNext())), but it is ineffective because some commands are fast to execute and others take time and resources.

CorellianAle
  • 645
  • 8
  • 16
  • 1
    Ya, I'd definitely use a map here. A mapping between, for example `TasksEnum.Three` and `executeTaskThree` would work fine. If you have a ton of commands, a map lookup might actually be faster than `if...else`s since it doesn't need to do a linear search over every command. – Carcigenicate May 07 '18 at 12:21
  • You can pass a pointer to function. – user7860670 May 07 '18 at 13:02

2 Answers2

1

I do believe there exists a better way to implement similar code.

A switch statement is as good as you can get. It will (usually) generate a jump table in your machine code that will directly send the program counter to the location to continue executing.

Using a map (red-black tree) to store ids and pointers to functions, is not as efficient as perfect hashing as in switch, and might prevent inline optimizations.

I would execute it in a background thread, meaning executeTaskTwo() would return nearly immediately.

I am not very knowledgeable about multi-threading, but why would you return early?

Isn't the point of a thread to join the rest of the threads after it finishes?

If what you mean is that the primary thread to continue to do other stuff, then you could have an std::atomic<bool> that tells you whether the process has been finished. If only one thread writes on it (working thread), while others read (main thread), the behavior is well defined.

If you don't want to store a bunch of booleans and simply want to know whether a process is still running, you could have a counter, which you increment and decrement as threads start and finish.

Kostas
  • 4,061
  • 1
  • 14
  • 32
  • I don't know how to describe more correctly, but the idea is the following: simple solutions are good when the amount of tasks is small, but there comes a time when these switch statements become absurdly big and hard to read. If a went with the `std::atomic` flag way, it would mean that each task would need a corresponding flag and a `if(flag)` statement somewhere, making the amount of code really hard to maintain. – CorellianAle May 07 '18 at 13:30
  • @CorellianAle In my opinion `switch` statements are very readable, even for long sequences. Especially if you put the break on the right of the function call, but this is your call. Check this [SO question](https://stackoverflow.com/questions/9094422/how-to-check-if-a-stdthread-is-still-running) on how to check if a thread is still running. – Kostas May 07 '18 at 13:36
0

Can you have Task as base class and let the `getNextFromQueue() return a concrete class derived from it? Here is a simple code on fly to give idea/

class Task
{
public:
void execute() = 0;

}

class TaskOne : public Task
{
public:
void execute()
  {
     // execute the task here
  }
}

So your code will look like:

Task * task = _taskProvider.getNextFromQueue();

if (task)
   task->execute()
zar
  • 11,361
  • 14
  • 96
  • 178