3

I'm in the process of converting a PHP project to Hack, and I've come across a bit of a road block. What I'm trying to do is rewrite a IoC container from PHP to Hack, and I'm having a little trouble getting everything to pass the Hack type checker tool.

So basically what I have is a container that lets you register string to closure mappings. The idea is the closure contains the logic to instantiate a class. The container also stores the instances it creates, and also allows you to force a creation of a new instance. Here is my container code:

<?hh // strict
class Container {
    private Map<string, mixed> $instances = Map {};
    private Map<string, (function (Container): mixed)> $registered = Map {};

    public function register(string $alias, (function (Container): mixed) $closure): void
    {       
        $this->registered[$alias] = $closure;
    }

    public function get(string $alias): ?mixed
    {
        if (!$this->registered->contains($alias)) {
            return null;
        }

        $instance = $this->instances->get($alias);
        if ($instance !== null) {
            return $instance;
        }

        $closure = $this->registered->get($alias);
        if ($closure !== null) {
            $this->instances->set($alias, $closure($this));
        }
        return $this->instances->get($alias);
    }

    public function getNew(string $alias): ?mixed
    {
        if (!$this->registered->contains($alias)) {
            return null;
        }

        $closure = $this->registered->get($alias);
        return ($closure !== null) ? $closure($this) : null;
    }
}

This class itself seems to pass the type checker, but when when using Container::get() or Container::getNew(), since the return is of type mixed, it throws this error when I try to execute a method on an object returned by these:

You are trying to access the member x but this is not an object, it is a mixed value

Now I know this makes sense, because mixed obviously allows for non-objects, so I would have to make sure to wrap such code in a is_object(), however doing this doesn't seem to suppress the error in the type checker. Is there a better way to confirm something is an object in Hack, that the type checker will understand?

Also, this IoC container class relies heavily on the mixed type, which is a bit ugly to me. It isn't ideal to have to make sure its returns are objects at run time either. Is there a better way I could be doing this? I did try playing with the idea of changing mixed to an interface (like IContainable or something), and have any class I want to store in the container implement this, but then the type checker complained that the IContainable interface didn't contain the method I was trying to call on the object returned from the container (so an error at the same point in code, but for a different reason). Perhaps I was close to success with this approach?

Thanks for any help.

ndavison
  • 185
  • 7

2 Answers2

10

Is there a better way to confirm something is an object in Hack, that the type checker will understand?

You probably want to check for a specific instance via instanceof. For example: (warning, code typed directly into browser)

<?hh // strict

class C {
  public function f(): void {}
}

function f(): mixed {
  return new C();
}

function g(): void {
  $c = f();
  // $c->f() won't work out here, $c is mixed
  if ($c instanceof C) {
    $c->f(); // This works now
  }
}

Also note that you don't want ?mixed, that's redundant -- just use mixed. (Since mixed can be anything, it can also be null. Generating an error when you write ?mixed is on my list of things to do at some point. :))

Also, this IoC container class relies heavily on the mixed type, which is a bit ugly to me.

Your intuition is good here. At Facebook, we've found that in most of the cases where we want to type something as mixed, either 1) there is a more specific type we should write, or 2) the code is structured very poorly and we should rethink how to structure it to be better typed.

Maybe you should use a more specific type. Using an interface, as you indicate, will help. You can couple it with the instanceof functionality described above if it turns out that most of the things you need to do are actually part of that interface, and you just very occasionally need to do something else. You might also want to take a look at using generics, which may or may not make sense, depending on how exactly you use your Container. If Container is instantiated many times, each for a different specific subclass or interface, then generics are probably just what you want. If it's a singleton, then perhaps maybe not. (Also, although you don't show it here, if Container interacts with the classes it holds in any way, you may need a constraint on the generic.)

But I suspect that you are running into this issue due to a structural problem. Here are some of the things that I think about when I run into this: Why do you need this layer of abstraction around instantiating classes in the first place -- what functionality is it providing, and can you make that functionality homogeneous (i.e., typable) and separate from the heterogeneous class instantiation problem? Ignoring the formal types for a moment (i.e., if you were writing this in vanilla PHP), how does the class that uses Container know it's getting the right thing? There has to be some knowledge of what is going in and out, however informal or otherwise tied up in the calling code, otherwise this wouldn't work in the first place! Think about where and how that information is codified, and then see if you can figure out a way to make that explicit.

Hope this helps! Let me know in comments if I can clarify anything. And of course, feel free to keep asking SO questions, a lot of FB Hack engineers as well as knowledgeable community members watch the "hacklang" tag closely :) (We're really excited that folks are trying Hack BTW! Hope you're enjoying it.)

Josh Watzman
  • 7,060
  • 1
  • 18
  • 26
  • +1 Great answer ! And many thanks for monitoring the "hacklang" and "hhvm" tags here on SO :) . – Radu Murzea May 10 '14 at 18:26
  • Awesome answer! I did look at generics, but the container acts as a singleton currently so I don't see that working. I added this layer because I like to code with DI and wrapping logic around how a class and its dependencies is instantiated seems to make sense. As for how it worked in PHP, I guess technically the PHP code never knew for sure it was getting the right thing, it was just a matter of the programmer knowing what alias was assigned to what class instantiation, and if a mistake was made it would be caught at run time. So not just a conversion here, but a rethink as well. – ndavison May 11 '14 at 01:10
0

I know I'm 6 months late, but I'd like to offer my solution to this problem.

I got around this issue by creating a script that will scan the project for factory methods and compiles them into a single class.

I achieved this by scanning for the user defined property provides which takes two arguments: the alias for the factory and the full name of the type of object the factory produces. The compile script also makes sure that each factory only takes a single parameter, which is the compiled factory class.

In essence instead of populating the container at run time, I populate it at compile time. The container class definition IS the container, rather than an instance of the container class being the container.

It's still a work in progress, but you can see my solution on github