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

Filtering Gantt chart data involving hierarchical DTOs

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

Problem

I have a bunch of code that deals with modifying a large nested set of IEnumerable

Basically, this is the structure of the DTO's

public class ProductLineDto
{ 
    ...
    public IEnumerable ProjectType { get; set; }
}
public class ProjectTypeDto
{
    ...
    public IEnumerable Projects { get; set; }
}
public class ProjectDto
{
    ...
    public IEnumerable SubProjects { get; set; }
}

public class SubProjectDto
{
    ...
    public IEnumerable Children { get; set; }
    public IEnumerable Activities { get; set; }
}


Here is what the code that uses them looks like.

-
I was wondering if there was a way to clean up the code. I don't like to have to make a copy of every nested IEnumerable to modify the ganttData variable. I was thinking of maybe making an extension method take care of that for me, that way I wouldn't have to throw those two lines of code in for every IEnuemerable

-
Are there any performance enhancements I can make? For instance, would it be faster to use Select() or Where() instead of RemoveAll()?

```
public IEnumerable FilterGanttChartData(IEnumerable ganttData, GanttFilterDto ganttFilterDataModel)
{
...

ganttData = GetDataByEndDate(ganttData.ToList(), ganttFilterDataModel);

return ganttData;
}

private IEnumerable GetDataByEndDate(List ganttData, GanttFilterDto ganttFilterDataModel)
{
if (ganttFilterDataModel.EndDateFrom == null && ganttFilterDataModel.EndDateTo == null)
return ganttData;

var gdata = new List();
gdata.AddRange(ganttData);

for (var i = 0; i ();
pType.AddRange(gdata[i].ProjectType);

for (var j = 0; j ();
projects.AddRange(pType[j].Projects);

for (var a = 0; a ();
subProjects.AddRange(projects[a].Children);
subProjects = GetActivitiesByEndDateRange(subProjects, ganttFilterDataModel.EndDateFrom, ganttFilterDataModel.EndDateTo).ToList();

Solution

Direction

I tried to rewrite your original code without much luck. After making a little bit of analysis I came to the idea that in order to have a more generic project shaking algorithm, we need a more generic project representation (data structure). Here's what the code looks after my second attempt.

Code

Here's the project tree object that has arbitrary number of subproject children defined as the same type as project itself (see Composite Pattern). The type placeholder T is something that will need some time to design properly, and this we can address separately.

Also, see that we've got the Activities enumerable declared on this level. Which means that even the root of the project hierarchy may have some activities. This is good and bad at the same time: it's good because now there's no artificial limitation applied to the root level; but it's bad because it diverges from what you had originally and if the original limitation is a business rule, you will need an external mechanism to maintain the rule.

public interface IProjectTree
    : IEnumerable> // !!! The implementation should rely on Children field
{
    IEnumerable> Children { get; set; }
    IEnumerable Activities { get; set; }

    T SubtreeSpecificData { get; set; }
}


Now we can build a Traverse class that implements a generic method ShakeOff which is generalized and can be applied to any part of the tree and with any thinkable predicate on activity object (passed as a parameter).

public static class Traverse
{
    public static IProjectTree ShakeOff(this IProjectTree targetTree, Func activityPredicate)
    {
        if (!targetTree.Any())
        {
            targetTree.Children = new List>();
        }
        else
        {
            targetTree.Children = targetTree
                .Children
                .Select(subTree => subTree.ShakeOff(activityPredicate))
                .ToList();
        }

        targetTree.Children = targetTree.Children.Where(subtree => subtree.ActivityPredicateApplies(activityPredicate));

        return targetTree;
    }

    public static bool ActivityPredicateApplies(this IProjectTree target, Func activityPredicate)
    {
        target.Activities = target.Activities.Where(activityPredicate);

        var hasSubtreesAndUnfilteredActivities = target.Any() && target.Activities.Any();
        return hasSubtreesAndUnfilteredActivities;
    }
}


Consumption

The consumer will then look like this:

DateTime fromEndDate = ...;
DateTime toEndDate = ...;
Func activityPredicate = activity =>
     activity.EndDate >= fromEndDate && activity.EndDate  projectTree = null;
projectTree = projectTree.ShakeOff(activityPredicate);


Side notes

But wait, I can't change my DTO classes into IProjectTree. I am pretty sure your DTO structure is shaped by the Database or the service and you want to keep them as they are. This is the hardest part, and you may not like the answer.

You will need to translate your DTOs into IProjectTree. It means, all your DTOs stay unchanged; add the ProjectTree type; and write a class that translates one thing into another (and vice versa). This will require a bit of work. And you will need to find a proper way to define the ` (the trivial solution would be an object, the SubtreeSpecificData could then point to the source DTO ;) ).

IMO, this would be a better design. Operating directly on DTOs is counter-intuitive to me. DTOs are not supposed to be used as a part of the domain logic, but rather as a dummy objects in client-server communication processes.

Disclaimer

  • Traverse.ShakeOff() method is not a pure function, it modifies the provided targetTree object in-place. This may be dangerous in some scenarios (but probably, not in your case).



  • The code above may or may not be easy to apply due to architectural changes -- it's your judgement call. If I were you, I would not limit myself to a rigid unmanageable data structure just because my database/service returns DTOs in this format. As mentioned earlier, generic representation enables generic algorithms which are used a LOT (meaning the effort pays off).



  • I didn't have much time to test the solution (in fact, I still don't have access to my compiler), so burden of testing is on you.



Update 1

I updated code as per t3chb0t's comment and own observations:

  • Added IEnumerable> to IProjectTree.



  • Replaced .Count() > 0 with .Any() where applicable.



  • Using extension method subTree.ShakeOff(activityPredicate)` instead of an explicit static method invocation.

Code Snippets

public interface IProjectTree<T>
    : IEnumerable<IProjectTree<T>> // !!! The implementation should rely on Children field
{
    IEnumerable<IProjectTree<T>> Children { get; set; }
    IEnumerable<ActivityDto> Activities { get; set; }

    T SubtreeSpecificData { get; set; }
}
public static class Traverse
{
    public static IProjectTree<T> ShakeOff<T>(this IProjectTree<T> targetTree, Func<ActivityDto, bool> activityPredicate)
    {
        if (!targetTree.Any())
        {
            targetTree.Children = new List<IProjectTree<T>>();
        }
        else
        {
            targetTree.Children = targetTree
                .Children
                .Select(subTree => subTree.ShakeOff(activityPredicate))
                .ToList();
        }

        targetTree.Children = targetTree.Children.Where(subtree => subtree.ActivityPredicateApplies(activityPredicate));

        return targetTree;
    }

    public static bool ActivityPredicateApplies<T>(this IProjectTree<T> target, Func<ActivityDto, bool> activityPredicate)
    {
        target.Activities = target.Activities.Where(activityPredicate);

        var hasSubtreesAndUnfilteredActivities = target.Any() && target.Activities.Any();
        return hasSubtreesAndUnfilteredActivities;
    }
}
DateTime fromEndDate = ...;
DateTime toEndDate = ...;
Func<ActivityDto, bool> activityPredicate = activity =>
     activity.EndDate >= fromEndDate && activity.EndDate < toEndDate;

IProjectTree<...> projectTree = null;
projectTree = projectTree.ShakeOff(activityPredicate);

Context

StackExchange Code Review Q#159778, answer score: 2

Revisions (0)

No revisions yet.