Remove static factories that match constructor

[ellisv]

This is to launch a discussion. So a question is whenever there is a reason for having static factories that match a constructor? There is a perfectly fine built in language syntax for creating objects and by introducing static factories we are giving users to doubt if to go with static factory or a new construct.

[ellisv]

@jcchavezs Any thoughts on this?

[localheinz]

@ellisv

I think there are perfectly valid use cases for named constructors - also see http://verraes.net/2014/06/named-constructors-in-php/.

What do you think?

[ellisv]

I believe the argument in the article to go for static factories was made to cover for lack of multiple construct support in PHP. But we do not have such problem in here. Never-the-less having such factories in our case does not give any benefits, in my humble opinion, but rather expands API surface with factory methods and gives more confusion to users - do I use a factory or construct with the new construct.

[piotrooo]

I'm agree with @ellisv about never-the-less. There are many places where we don't need that pattern (workaround).

Examples where this approach is over engineering:

• https://github.com/opentracing/opentracing-php/blob/42eb0e58c68054fd7c994b58831ea4cc207e8a0e/src/OpenTracing/NoopScope.php#L7
• https://github.com/opentracing/opentracing-php/blob/42eb0e58c68054fd7c994b58831ea4cc207e8a0e/src/OpenTracing/NoopScopeManager.php#L7
• https://github.com/opentracing/opentracing-php/blob/42eb0e58c68054fd7c994b58831ea4cc207e8a0e/src/OpenTracing/NoopSpan.php#L7
• https://github.com/opentracing/opentracing-php/blob/42eb0e58c68054fd7c994b58831ea4cc207e8a0e/src/OpenTracing/NoopSpanContext.php#L9
• https://github.com/opentracing/opentracing-php/blob/42eb0e58c68054fd7c994b58831ea4cc207e8a0e/src/OpenTracing/Reference.php#L48

But in that cases:

• https://github.com/opentracing/opentracing-php/blob/42eb0e58c68054fd7c994b58831ea4cc207e8a0e/src/OpenTracing/Mock/MockSpanContext.php#L38
• https://github.com/opentracing/opentracing-php/blob/42eb0e58c68054fd7c994b58831ea4cc207e8a0e/src/OpenTracing/Mock/MockSpanContext.php#L43
• https://github.com/opentracing/opentracing-php/blob/42eb0e58c68054fd7c994b58831ea4cc207e8a0e/src/OpenTracing/Mock/MockSpanContext.php#L49

looks like good usage of mulit-constructor in PHP. So removing of that "factories" should be only on that unnecessary places.

[piotrooo]

@jcchavezs what is the current approach? I think we don't need it in all cases.

[jcchavezs]

I think this is a valid concern. Would you @piotrooo be up to rebase this so we can get it merged?

[piotrooo]

But you think we should use in all places, that static method? Or just in a that places where I thought is necessary?

[jcchavezs]

i left a comment. All the rest can be changed.

[jcchavezs]

I left a comment. All the rest can be kept as it is in this PR @piotrooo (unless @ellisv has something to say)