Jump to content
Sign in to follow this  
Mike Torrettinni

Maintaining For loop(s)

Recommended Posts

I'm looking for general suggestion on how to deal with this problem:


I have array of Project:

TProject = record

  ID: integer;

  Name, Description: string;

  Active, HasSubProject: boolean;

  WorkHours: integer;

  ...

end;

Projects: TArray<TProject>;

 

It has 50+ fields. Now I need a lot of different information about projects, based on criteria on fields. For example count all Active project, or Count all Projects with WorkHours >= 200, or list all Projects with HasSubProjects...

So, basically a lot of For loops in different methods, used in different units.

 

The problem I always face is that when adding new Field, like International: boolean flag which affects all my For loops, how to make sure I don't forget to use it in one of my existing For loops.

I always search where Projects are used and modify all For loops as needed, but I noticed I can miss some For loops, especially when IDE doesn't always find the usage in all units. And then I have For loop that is not current anymore.

 

So, one of the idea is to have ONLY 1 For loop for Projects and add all criteria needed with If statements, so I will have 1 For loop to maintain - it will be a lot of IFs, but at only 1 place.

 

Is this good design? Any other suggestions how you deal with this?

 

 

Share this post


Link to post

I would serialize the whole filtering, one by one, always returning with the matches and do the next filter (for loop) on the result. (If not already empty).

 

Share this post


Link to post
Guest

You can use the Specification Pattern within a single loop.

You will find a Delphi implementation at Spring4D

 

Here a sample how you can build a specification

var
  isActive, hasSubprojects, projectSpecification: Specification<TProject>;
  workhoursGreaterThan: TFunc<integer, Specification<TProject>>;
begin
  isActive := Specification<TProject>(
    function(const p: TProject): Boolean
    begin
      Result := p.active;
    end);

  hasSubprojects := Specification<TProject>(
    function(const p: TProject): Boolean
    begin
      Result := p.active;
    end);

  workhoursGreaterThan := function(v: integer): Specification<TProject>
    begin
      Result := Specification<TProject>(
        function(const p: TProject): Boolean
        begin
          Result := p.WorkHours > v;
        end);
    end;

  projectSpecification := (isActive and not hasSubprojects and workhoursGreaterThan(20)) or 
    (not isActive and workhoursGreaterThan(50));

end;

 

Edited by Guest

Share this post


Link to post

Instead of

 

Projects: TArray<TProject>

 

use a list class:

type
  TProjectList = class(TList<TProject>)
    
  end;  

Add public methods to the class that implement your current for loops. This way you have all this code centralized in one place, which should massively simplify the maintenance.

And while you're at it: convert the record into a class. Lists of records are a bit inefficient since accessing the items in the list involves a lot of copying of data. You can then use a TObjectlist<TProject> as base class for your TProjectlist and let the list manage the memory of the TProject objects you add to it (OwnsObjects parameter for the list constructor set to true, which is the default).

 

  • Like 2

Share this post


Link to post

Take a look at FireDAC Local SQL engine. You can populate FDMemTable with data stored in your TProject records and run plain SQL queries against this dataset.

Share this post


Link to post

The fundamental problem sounds like you have duplication of code. Attack that and the problem you describe simply disappears. 

  • Like 1

Share this post


Link to post

Thank you, interesting suggestions. Never heard of Specification Patterns, seems like something I can do for next big data container. Probably doesn't make sense for current Projects as it completely changes everything.

@Attila Kovacs: are you referring to the same thing, Specification patterns?

 

@PeterBelow Yes, this could be next step, to organize all methods related to Projects into single unit, I guess into a class, like your example. This will keep all my For loops in one place. And when change is needed, I can quickly browse through methods and see For loops.

 

@Dmitriy M I think this is overkill, using any kind of SQL engine for my needs. I don't really see benefit of writing sql statements over For loops - at the end it is a method you call. Right?

 

@David Heffernan Yes, I have numerous methods like CountActiveProjects(), IsProjectActive(), ListActiveProjects... well, each only one, but they are all over the project, as I need different information on different screens, in different units. 

 

 

 

 

Share this post


Link to post

Yes, that is part of my years long project refactoring. And now Projects array is what I'm working on, so I noticed a lot of methods (non duplicated) that return information (counts, lists, exist/not exist,,,) based on different criteria. So a lot of For loops on the same array.

 

 

Share this post


Link to post
On 1/19/2019 at 2:06 AM, Schokohase said:

You will find a Delphi implementation at Spring4D

Thanks for pointing that out - in the currently released version the type still has a T in its name and is in the unit Spring.Designpatterns (the move and renaming happens in the upcoming 1.3 version which is not yet released).

Also in both versions the explicit hardcasting of the anonymous method is not required - there is an implicit operator for that.

 

Also FWIW I usually put specifications that are used in several places as static functions into their own type or helper just like this (you can also make them standalone routines if you dislike the redundant typing of TProjectFilters):

 

type
  TProjectFilters = record
    class function IsActive: TSpecification<TProject>; static;
    class function WorkhoursGreaterThan(const value: Integer): TSpecification<TProject>; static;
  end;

class function TProjectFilters.IsActive: TSpecification<TProject>;
begin
  Result :=
    function(const p: TProject): Boolean
    begin
      Result := p.Active;
    end;
end;

class function TProjectFilters.WorkhoursGreaterThan(
  const value: Integer): TSpecification<TProject>;
begin
  Result :=
    function(const p: TProject): Boolean
    begin
      Result := p.Workhours > value;
    end;
end;

var
  spec: TSpecification<TProject>;
begin
  spec := TProjectFilters.IsActive and TProjectFilters.WorkhoursGreaterThan(50);

Also what others already pointed out: lets look at for example CountActiveProjects and ListActiveProjects - what do they have in common? They do something on the active projects - so break that down into getting the active projects and what is done on them. If you want to keep TArray<TProject> and alike you probably have to write the filtering yourself. I just point out how easy it will be using IList<T> from Spring.Collections:

 

procedure PrintProject(const p: TProject);
begin
  //...
end;

var
  projects: IList<TProject>;
  activeProjects: IEnumerable<TProject>;
begin
  activeProjects  := projects.Where(
    function(const p: TProject): Boolean
    begin
      Result := p.Active;
    end);
  Writeln(activeProjects.Count);
  activeProjects.ForEach(PrintProject);

You can even turn a TArray into an IEnumerable where you can apply filtering and stuff (yes, some minor overhead - measure yourself if it's worth it to keep your code compact and with less explicit loops);

 

var
  projects: TArray<TProject>;
  activeProjects: IEnumerable<TProject>;
begin
  activeProjects  := TEnumerable.From(projects).Where(
    function(const p: TProject): Boolean
    begin
      Result := p.Active;
    end);
  Writeln(activeProjects.Count);
  activeProjects.ForEach(PrintProject);

 

Edited by Stefan Glienke

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
Sign in to follow this  

×