5
\$\begingroup\$

Make() takes projects with a list of tuples representing project references in the form of (Dependency, Dependent) and returns projects in the order to be build:

static void Main(string[] args)
{
    var projects = new[] { "a", "b", "c", "d", "e", "f" };
    var dependencies = new[] { ("a", "b"), ("a", "c"), ("e", "f"), ("c", "f"), ("f", "d") };
    Console.WriteLine(
        string.Join(", ", Make(projects, dependencies))); // a, e, b, c, f, d
}

Where:

static IEnumerable<string> Make(
    IEnumerable<string> projects, 
    IEnumerable<(string Dependency, string Dependent)> references)
{
    var dependents = new HashSet<string>(references.Select(d => d.Dependent));
    var build = new HashSet<string>(projects.Except(dependents));
    return 
        !projects.Any() ? Enumerable.Empty<string>() :
        !build.Any() ? throw new Exception("Circled references detected.") :
        build.Concat(Make(
            projects.Except(build).ToArray(),
            references.Where(r => !build.Contains(r.Dependency)).ToArray()));
}

What about run-time complexity of this one? Could it be optimized?

\$\endgroup\$
3
  • \$\begingroup\$ I looks like there is already an official algorithm for that. See Nikita B's answer to a similar question of mine. However, not so compact. Although I think it could be improved. \$\endgroup\$
    – t3chb0t
    Aug 26, 2019 at 8:06
  • 2
    \$\begingroup\$ @t3chb0t Yep, I just tried to be immutable and as concise as possible (as usual :), it is almost the same. There is one in Cracking coding interview on page 250 (common, everybody has this book :) - but there it looks really, really scary :) \$\endgroup\$ Aug 26, 2019 at 8:12
  • 1
    \$\begingroup\$ You probably should add a notice in your questions that reviews should favor functional solutions ;-) \$\endgroup\$
    – t3chb0t
    Aug 26, 2019 at 8:51

1 Answer 1

8
\$\begingroup\$

Exit early

Instead of stuffing everything in one return statements, return as early as possible.

if (!projects.Any()) return Enumerable.Empty<string>();

can be the first statement of your method. It saves recreating the hashset objects and it makes the flow of your method with its recursion more clear. I think sacrificing a bit of "conciseness" for readability is worth it.

ToArray

Are unnecessary and cause additional loops over your sets.

Except and HashSet

Enumerable.Except is already implemented using sets. The second argument is added to a set and the first is streamed through it. Creating the HashSet manually is a wasted effort. See the reference source.

So dependents can just be:

var dependents = references.Select(d => d.Dependent);

Contains and HashSet

Contains on the other hand benefits from the HashSet overload, since the Enumerable.Contains implementation just loops over the sequence. So build should be a HashSet to cope with larger inputs. While we're at it, we're using Linq, so ToHashSet().

static IEnumerable<string> Make(
    IEnumerable<string> projects,
    IEnumerable<(string Dependency, string Dependent)> references)
{
    if (!projects.Any())
    {
        return Enumerable.Empty<string>();
    }
    var dependents = references.Select(d => d.Dependent);
    var build = projects.Except(dependents).ToHashSet();
    if (!build.Any())
    {
        throw new Exception("Circled references detected.");
    }
    else
    {
        return build.Concat(Make(
              projects.Except(build),
              references.Where(r => !build.Contains(r.Dependency))));
    }
}
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.