Config merge is too late

[Sylry]

I think that the config merge in HtmlConverter::__construct() happens too late because the setConfig() of ConfigurationAwareInterface converters has already be called with the default config.

It will allow to set some properties on the object once at setConfig() and to re-use these properties in convert() without executing several times the code to set those properties

PR will follow

[Sylry]

PR: #152

[colinodell]

The idea with the Configuration class is that your converter object should store the Configuration instance and call $this->config->get('whatever') during the convert() method. It should not pull out those values immediately during the setConfig() method. This means that config merging is never too late, since you aren't pulling the values out until after the merge has occurred.

I'm not necessarily opposed to changing how the configuration works. However, I'm not completely sold on the implementation in #152 for a couple reasons:

1. It's already possible to pull the most-merged values out, so this isn't necessarily "fixing" anything, but rather making a change
2. The change is a breaking change which would require a major version bump. I'm not convinced this change is significant/useful enough to warrant a BC-break.

That being said, I'm definitely open to hearing your thoughts - you're welcome to try changing my mind :)

I appreciate you taking the time to share your thoughts and proposal!

[Sylry]

Thanks for the explanation.

The problem I see with that option is that if you need to execute some code depending on the Config, you will execute this code each time you convert an HTML fragment.
Imagine this code is expensive and you will experience performance problems.

I have a case in a import: I instanciate once the HtmlConverter and for each piece of content in my source, I use it to convert the HTML in MD.

I need it for #154 to work, but will try different approach

[colinodell]

Okay, I see where you're coming from. I would still like to keep the current functionality as-is though.

For your use case, I'd recommend doing something like this:

class HtmlConverter implements ConverterInterface, ConfigurationAwareInterface
{
    private $foo; // null until convert() is called

    public function convert(ElementInterface $element)
    {
        if ($this->foo === null) {
            $this->foo = new Whatever(); // lazy-instantiate whatever you need here based on the config
        }

        // $this->foo will always be the same instance now
    }
}