Welcome to the Lounge

Sander Rossel wrote:

Only as you said, should do one thing vs. should do x things, but that's something that changes with specs, and both can be easily changed.
The difference is that when the specs are changed to "if x do y and now also z" the code with braces is almost certain to be changed correctly, but without braces that change is more likely to cause a bug.

//if (x)
//   DoSomething(); Commented out for now.
AlwaysDoThis();

if (x)
{
//   DoSomething(); Commented out for now.
}
AlwaysDoThis();

if (x)
{
   if (y)
      DoSomething();
   else
      DoSomethingElse();
}

							}
						}
					}
				}
			}
		}
        ++i;
	}
}

void ProcessThing(Thing thingy)
{
   try
   {
      ProcessFoos(thingy.Foos);
   }
   catch (Exception ex)
   {
      // Handle error.
   }
}

void ProcessFoos(List<Foo> foos)
{
   foreach (var foo in foos)
   {
      ProcessBars(foo.Bars);
   }
}

void ProcessBars(List<Bar> bars)
{
   foreach (var bar in bars)
   {
      ProcessStuff(bar.Stuff);
   }
}

// ...

static void _FillLRClosureInPlace(CfgDocument cfg,IProgress<CfgLalr1Progress> progress, ICollection<LRItem> result)
{
	var done = false;
	while (!done)
	{
		done = true;
		var l = result.ToArray();
		for (var i = 0; i < l.Length; ++i)
		{
			if(null!=progress)
				progress.Report(new CfgLalr1Progress(CfgLalr1Status.ComputingClosure, i));
			var item = l[i];
			var next = item.RightIndex < item.Rule.Right.Count ? item.Rule.Right[item.RightIndex] : null;
			if (item.RightIndex < item.Rule.Right.Count)
			{
				if (cfg.IsNonTerminal(next))
				{
					for (int jc = cfg.Rules.Count, j = 0; j < jc; ++j)
					{
						var r = cfg.Rules[j];
						if (r.Left == next)
						{
							var lri = new LRItem(r, 0);
							if (!result.Contains(lri))
							{
								done = false;
								result.Add(lri);
							}
						}
					}
				}
			}
		}
	}
}

static void _FillLRClosureInPlace(CfgDocument cfg, IProgress<CfgLalr1Progress> progress, ICollection<LRItem> result)
{
    var done = false;
    while (!done)
    {
        done = true;
        var l = result.ToArray();
        for (var i = 0; i < l.Length; i++)
        {
            progress?.Report(new CfgLalr1Progress(CfgLalr1Status.ComputingClosure, i));
            var item = l[i];
            var next = item.RightIndex < item.Rule.Right.Count ? item.Rule.Right[item.RightIndex] : null;
            if (item.RightIndex < item.Rule.Right.Count && cfg.IsNonTerminal(next))
            {
                done = HandleRules(cfg, next, result);
            }
        }
    }
}

private static bool HandleRules(CfgDocument cfg, object next, ICollection<LRItem> result)
{
    bool done = true;
    // cfg.Rules.Count doesn't change, does it?
    for (int j = 0; j < cfg.Rules.Count; j++)
    {
        var r = cfg.Rules[j];
        done = HandleRule(r, next, result);
    }
    return done;
}

private static bool HandleRule(Rule r, object next, ICollection<LRItem> result)
{
    bool done = true;
    if (r.Left == next)
    {
        var lri = new LRItem(r, 0);
        // I think this communicates intent more clearly.
        done = result.Contains(lri);
        if (!done)
        {
            result.Add(lri);
        }
    }
    return done;
}

for(var i = 0;i<list.Count;++i);

for(int ic=list.Count,i=0;i<ic;++i);

honey the codewitch wrote:

Because look carefully. In that loop I'm modifying the passed in collection.

honey the codewitch wrote:

Yes I really like foreach, and I use it a lot. But not if I don't have to.

honey the codewitch wrote:

alse HandleRules is unnecessary

honey the codewitch wrote:

Literally all you factored was the loop itself.

if(!HandleRules(cfg,next,result))
   done=false;

honey the codewitch wrote:

If you're smart, that's how you'll take them.

honey the codewitch wrote:

Actually the API is easy to use. It's just big.