Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PHP7] Added support for abstract final classes #923

Closed

Conversation

8 participants
@guilhermeblanco
Copy link

commented Nov 25, 2014

Consumes the PR #911

Abstract final classes are helpful in the case you are wrapping common functions that are static, but the common class itself cannot be instantiated.
Currently, PHP developers' only resource is to create a final class with a private constructor, leading to untestable and error prone code.

For such, here is motivation:

  • As "abstract", it cannot be instantiated
  • As "final", it cannot be extended (such as visibility increase, behavior change, etc)
  • There's no way of adding variables to a namespace. This would address this issue too
@lstrojny

This comment has been minimized.

Copy link
Contributor

commented Nov 26, 2014

Shouldn’t this just be a static class?

@Tyrael

This comment has been minimized.

Copy link
Member

commented Nov 26, 2014

I agree with @lstrojny here, you are right in the sense that abstract + final satisfies your condition (a class which can't be instantiated nor extended) but that's already possible without this change(as you mentioned you can create a final class with a private constructor).
Also, I fail to see how would this be a better alternative (you mentioned testability for example, as far as I can tell final + abstract would be equally untestable as final+private constructor) plus it would be a bit stretching the definition of the abstract modifier, as that is about a contract/requirement to the subclasses.
Using that together with final which explicitly prohibits the existence of subclasses is confusing/conflicting idea and I can't even remember a single language which supports that combination(from the top of my head).

@guilhermeblanco

This comment has been minimized.

Copy link
Author

commented Nov 27, 2014

@lstrojny Shouldn’t this just be a static class?

Conceptually, the "static" operator means that consecutive calls always return same value/instance. This means that at a class level it would return same instance at multiple "new" calls (read as singleton).

@Tyrael Parts I already answered previously. This patch is currently missing the enforcement for "static" existence on properties and methods, which I'm almost done.
Now there is a language that does that... Hack. =) Here is the commit that introduces this support, which I happened to be pointed out by a friend today: facebook/hhvm@faedfaf
Also, C# supports the same configuration. I added the same enforcement both Hack and C# do (must be static members, covering both properties and methods).

@Hywan

This comment has been minimized.

Copy link
Contributor

commented Nov 27, 2014

@smalyshev smalyshev added the RFC label Nov 28, 2014

@guilhermeblanco guilhermeblanco force-pushed the guilhermeblanco:abstract-final-classes branch from 1a713c6 to 5e90b58 Dec 1, 2014

Guilherme Blanco added some commits Nov 22, 2014

Guilherme Blanco
Removed ZEND_ACC_FINAL_CLASS which is unnecessary. This also fixed so…
…me currently defined classes as final which were just not being considered as such before.
@smalyshev

This comment has been minimized.

Copy link
Contributor

commented Dec 6, 2014

@guilhermeblanco SInce the RFC is about static classes now, should this pull be closed?

@guilhermeblanco

This comment has been minimized.

Copy link
Author

commented Dec 6, 2014

@smalyshev I personally don't really care about which one we choose.
My original idea was to ask about "abstract final", matching HHVM implementation, but also consider C# "static class" too. Maybe we should leave both open, ask for 3 options to vote (static class, abstract final class or no support) and then close/merge whatever is appropriate.
What do you think?
We could have 2 questions. One is yay/nay for such of support and if positive, and the other to choose between the two approaches.

public static function display($name)
{
echo "Hello $name\n";
}

This comment has been minimized.

Copy link
@malukenho

malukenho Dec 6, 2014

Contributor

indentation here.

public static function display($name)
{
echo "Hello $name\n";
}

This comment has been minimized.

Copy link
@malukenho

malukenho Dec 6, 2014

Contributor

indentation here.

public function display($name)
{
echo "Hello $name\n";
}

This comment has been minimized.

Copy link
@malukenho

malukenho Dec 6, 2014

Contributor

same here.

@jpauli jpauli added the PHP7 label Dec 12, 2014

@Kubo2

This comment has been minimized.

Copy link
Contributor

commented Dec 14, 2014

@guilhermeblanco I am compliant with final abstract classes in PHP. In past, I also got this idea in mind but then I just threw it off.

@Kubo2 Kubo2 referenced this pull request Dec 14, 2014

Closed

[PHP7] Static classes #929

@guilhermeblanco

This comment has been minimized.

Copy link
Author

commented Dec 24, 2014

Feature got rejected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.