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

Improve my Task Loops

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

Problem

How can I improve up this code?

Is it ok to write code using Invoke and Action so liberally or is this bad?

Performance is not an issue as I'm not using Invoke 50,000x in a a row (a lot of other things will be done in-between, making the performance hit null).

The primary goal is to run my list of tasks either by Item first or task first. The secondary goal is to NOT write a massively long method where each method is written out in each way they can be completed. Doing so makes adding new tasks later much easier.

Code: (Updated with pseudo extras)

public class TaskObject
{
    public bool RunPerItem = true; //PLACEHOLDER, normally derived externally from view model
    public bool RunPerTask = false; //PLACEHOLDER, normally derived externally from view model

    public Dictionary() TaskItems = new Dictionary()
    {
        { "ItemA", new Item() {} }, 
        { "ItemB", new Item() {} }, 
        { "ItemC", new Item() {} }, 
    }   

    private List> Tasks = new List>()
    {
        (TO, I) => { TO.TaskA(I); },
        (TO, I) => { TO.TaskB(I); },
        (TO, I) => { TO.TaskC(I); }
    };

    public void RunTasks()
    {
        //RunPerItem & RunPerTask are type `bool`
        if (this.RunPerItem) 
        {
            foreach (var I in this.TaskItems)
            {
                foreach (var T in this.Tasks)
                {
                    T.Invoke(this, I.Value); 
                }
            }
        }
        else if (this.RunPerTask)
        {
            foreach (var T in this.Tasks)
            {
                foreach (var I in this.TaskItems)
                {
                    T.Invoke(this, I.Value); 
                }
            }
        }
    }  

    public void TaskA(Item I) { /*...*/ }
    public void TaskB(Item I) { /*...*/ }
    public void TaskC(Item I) { /*...*/ }
}

public class Item 
{
    public string ItemName { get; set; }
}


Thanks for any advice/help!!!

Edit/Clarification/Extra Details:

The a

Solution

I don't know if you can, depending on your real world needs... but if you can convert the TaskItems Dictionary to a List of KeyValuePairs like this...

public List> TaskItems = new List>
{
    new KeyValuePair( "ItemA", new Item() {} ),
    new KeyValuePair( "ItemB", new Item() {} ), 
    new KeyValuePair( "ItemC", new Item() {} ), 
};


Then you can use the ForEach() exstension method to reduce the loop code to...

if (this.RunPerItem) 
{
    TaskItems.ForEach(I => Tasks.ForEach(T => T(this, I.Value)));            
}
else if (this.RunPerTask)
{
    Tasks.ForEach(T => TaskItems.ForEach(I => T(this, I.Value)));
}


If you couple that with @Steven Jueris' idea of splitting them into their own methods, your code will be very clean.

Code Snippets

public List<KeyValuePair<string, Item>> TaskItems = new List<KeyValuePair<string, Item>>
{
    new KeyValuePair<string, Item>( "ItemA", new Item() {} ),
    new KeyValuePair<string, Item>( "ItemB", new Item() {} ), 
    new KeyValuePair<string, Item>( "ItemC", new Item() {} ), 
};
if (this.RunPerItem) 
{
    TaskItems.ForEach(I => Tasks.ForEach(T => T(this, I.Value)));            
}
else if (this.RunPerTask)
{
    Tasks.ForEach(T => TaskItems.ForEach(I => T(this, I.Value)));
}

Context

StackExchange Code Review Q#3652, answer score: 4

Revisions (0)

No revisions yet.