TreeKey and MultiKey shouldn't be mutually-exclusive.

[fengdai]

A key should be able to implement both TreeKey and MultiKey. Because the two interfaces are not logically mutually-exclusive.

For an instance. A NormalScreen and A DialogScreen, they both have parent key CommonPath. The implementation should be like this:

public class NormalScreen implements TreeKey {
    public NormalScreen() {
    }

   @Override public Object getParentKey() {
    return new CommonPath();
   }
}

public class DialogScreen implements MultiKey, TreeKey {
    final Object mainContent;
    public DialogScreen(NormalScreen mainContent) {
        this.mainContent = mainContent;
    }

   @Override public List<Object> getKeys() {
       return Collections.singletonList(mainContent);
   }

   @Override public Object getParentKey() {
    return new CommonPath();
   }
}

[Zhuinden]

MultiKey just means it can have multiple "parents", or at least "keys that exist at the same time".

Honestly, TreeKey is just a 1-element multikey.

[fengdai]

I agree that MultiKey means "keys that exist at the same time". But I don't agree that it can have multiple "parents". Multikey's keys are not it's "patents". KeyManager#setUp method also implicites this.
And I don't think MultiKey is a general case of TreeKey. The MultiKey-sample implicites that Multikey has nothing to do with Treekey. That's why I think a key should be able to implement both.

[Zhuinden]

I was planning to ramble about how they are the same thing, but they really aren't. MultiKey is missing an additional step.

  void setUp(Object key) {
    Services parent = managedServices.get(ROOT_KEY).services;
    if (key instanceof MultiKey) {
      for (Object part : ((MultiKey) key).getKeys()) {
        setUp(part);
      }
      ensureNode(parent, key).uses++;
    } else if (key instanceof TreeKey) {
      TreeKey treeKey = (TreeKey) key;
      final Object parentKey = treeKey.getParentKey();
      setUp(parentKey);
      parent = managedServices.get(parentKey).services;
      ensureNode(parent, key).uses++;
    } else {
      ensureNode(parent, key).uses++;
    }
  }

It would be the same if Multikey called

  ensureNode(part, key).uses++;

But it doesn't. In that case, it shouldn't be else if, I guess.

[fengdai]

That's what I mean.

[fengdai]

@loganj Any official explanation?

[loganj]

It's confusing and I'm not aware of any real world use case, so I punted.

[fengdai]

@loganj Do you mean that a DialogScreen like flow-sample-multikey's will never have parent key? But in real world, DialogScreen isn't always as simple as a "message dialog". Sometimes a dialog can have very complex UI and depend on a variety of resources. How can we share the resources with other screens without TreeKey?

[Zhuinden]

If they are mutually exclusive, then how are you supposed to have a master detail flow, where the detail has its own dialog in separate Key app state?

[loganj]

They're not mutually exclusive, exactly. MultiKeys are meant to compose horizontally, rather than vertically.

[MultiKey's] component keys may be ordinary objects, {@link TreeKey}s, or MultiKeys.

So maybe something like this:

public class Master implements TreeKey { /* ... */ }
public class Detail implements TreeKey { /* ... */ }
public class DialogContent implements TreeKey { /* ... */ }

public class MasterDetail implements MultiKey {
  public List<Object> getKeys() {
    return Arrays.asList(master, detail);
  }
}

public class Dialog implements MultiKey {
  public List<Object> getKeys() {
    return Arrays.asList(masterDetail, dialogContent);
  }
}

Maybe the fact that the MultiKey is itself treated as a key, and not just a container for keys, is creating more problems than it solves.

[fengdai]

Maybe the fact that the MultiKey is itself treated as a key, and not just a container for keys, is creating more problems than it solves.

I agree. Flow-sample-multikey module is confusing and should be updated.

[loganj]

Well, that's a behavior change. If MultiKeys shouldn't be treated as keys, I'd need to make some changes in KeyManager.

[loganj]

Coming around to the notion that MultiKeys should not be treated as keys, but just as history frames that hold a list of keys. This would simplify a lot.

[dcow]

@loganj I like that notion.

[loganj]

Closing this discussion, #207 seems like the way forward here.