Question: The code below just senses when the network is available and enables/disables a button on the UI. If I am registered to the `NetworkAvailability` event for the life of the program, will it cause a memory leak? I heard it might but didn't understand why.

```
System.Net.NetworkInformation.NetworkChange.NetworkAvailabilityChanged += new System.Net.NetworkInformation.NetworkAvailabilityChangedEventHandler(NetworkChange_NetworkAvailabilityChanged);

delegate void EnableCallback();

void NetworkChange_NetworkAvailabilityChanged(object sender, System.Net.NetworkInformation.NetworkAvailabilityEventArgs e)
{
    EnableSync();
    //throw new NotImplementedException();            
}

private void EnableSync()
{
    if (this.btnSync.InvokeRequired)
    {
        EnableCallback methodCallback = new EnableCallback(EnableSync);
        this.Invoke(methodCallback, new object[] { });
    }
    else
    {
        if (btnSync.Enabled == true)
            btnSync.Enabled = false;
        else
            btnSync.Enabled = true;
    }
}

```

Answer: 
This code is not ready for review, as it's not working—it visits some rules more than once. If you need help writing a [topological sort algorithm](http://en.wikipedia.org/wiki/Topological_sort), Stack Overflow is the right place, not Code Review.

But I'll make one important observation about your code: there is no documentation. It is not clear from your code what it is supposed to do and how to use it. When you have a function whose behaviour is complex, it's important to start by writing a specification for it: what inputs does it take, and what output does it produce?

Once you've written the specification, stop and think. Is this specification clear? Is it easy for people to understand and use? If the answers are "no", then think about how you could change the specification.

If you try to do this, then you'll immediately see the problem. I had a go myself and this is what I came up with:

```
def _yield_name_dep(rules_deps):
    """Generate rules from rules_deps, together with their dependencies,
    in topological order: that is, each rule is generated before any of 
    the rules it depends on.

    The argument rules_deps must be a dictionary mapping a rule 
    object R to either
    (i) the name of the (single) rule that R depends on; or
    (ii) a sequence of names of rules that R depends on; or
    (iii) None if R has no dependencies.

    The names of the dependencies must be names of objects in the rules
    module, each of which must have a "depends" attribute containing the
    names of the rule's dependencies (as above).

    The generated output consists of pairs (rule, dependencies), with
    rule being a rule object (or the rule name, if the rule has no          
    dependencies), and dependencies as above.
    """

```

Did I get this right? Who knows? This is a *terrible* specification. Imagine being a programmer trying to read the documentation and figure out how to use the function!

So you must start by simplifying and clarifying the specification. Design the interface so that is easy to understand and easy to use, and then fixing the code will be straightforward.
Variable names
==============

You mean the dependencies of a rule with `dep`, so use that as variable name. Variable names are a very important part of the documentation of the code. `dep` and `ii` are not so good variable names

`visited`
=========

To keep the visited nodes, I would use a `set`, since this is the container meant to hold distinct unordered entries.

I would also make this a parameter with default value `None`, and initialize it to `set()` if it isn't passed in the function. This simplifies the initial calling of the method.

`start_nodes`
=============

Why do you include the dependencies in this parameter? Just the node should suffice. The dependencies can be looked up in `rules`.

Since you call this function recursively, the nodes you pass in on the second call will no longer be start nodes, so I renamed this parameter to `nodes`

Control flow
============

To limit the number of indents, instead of `if rule not in visited`, I would use `if rule in visited: continue`

The second check `if dep not in visited:` is already incorporated in the cal to `_dfs`, so doesn't need to be repeated here.

The fact you don't have to look up the dependent roles to call `_dfs` anymore also greatly simplifies the recursive calls.

```
def _dfs(nodes, rules, visited=None):
    if visited is None:
        visited = set()
    for rule in nodes:
        if rule in visited:
            continue
        dependencies = rules[rule]
        yield rule, dependencies

        visited.add(rule)

        for item in _dfs(dependencies, rules, visited):
            yield item

```