This issue is very similar to #3150371: Vote/VoteInterface are documented improperly and incompletely only we're patching the VoteResult classes here. The patch is also going to be very similar because the VoteResult entity is almost identical to the Vote entity. Likewise for the interfaces. Please refer to the comments on that other issue first, if you have any questions about what is being done here.
VoteResultInterface exists to define the required public methods of vote result entities.
VoteResultInterface should be the place where these methods are defined and documented, because that is the purpose of the interface - to provide a public "contract" for the methods - their call parameters, their return types, and a description of what they are intended to do.
The whole notion of abstracting things to an interface is so alternative implementations may be provided and so those implementations will work as long as they are defined and perform according to the interface definition.
However, VoteResultInterface and VoteResult are coded improperly. Specifically:
- Most of the VoteResultInterface methods are documented with
{@inheritdoc}
This is wrong because VoteResultInterface does not inherit these methods from any other interface, so there is no documentation to "inherit". Likewise, VoteResultInterface is supposed to be DEFINING these methods, meaning this is the ONLY place they can be documented. Additionally, some of the documentation that is in VoteResultInterface is wrong - for example all the setters return $this but that is not documented, and there are some needed corrections. All of these{@inheritdoc}
comments need to be replaced with real and accurate method documentation. - VoteResultInterface extends EntityOwnerInterface, but it also REDEFINES all four of the the methods declared on EntityOwnerInterface. This is wrong. EntityOwnerInterface defines these methods and documents these methods, VoteResultInterface should NOT be redefining them - that defeats the whole purpose of extending EntityOwnerInterface and destroys the documentation provided by EntityOwnerInterface. These redeclarations need to be removed from VoteResultInterface.
VoteResult::setCreatedTime()
andVoteResult::getCreatedTime()
are an essential part of the VoteResult entity definition, but they are not part of VoteResultInterface. That is wrong. If these methods are not defined on VoteResultInterface that means we have no way of providing alternative implementations of Vote because there is no way to enforce the required functionality of these implementations. The definition of get/setCreatedTime() needs to be added to VoteResultInterface.
Comment | File | Size | Author |
---|---|---|---|
#7 | interdiff-4-7.txt | 330 bytes | TR |
#7 | 3151918-7-vote-result-interface.patch | 3.33 KB | TR |
|
Comments
Comment #2
TR CreditAttribution: TR commentedComment #3
Krzysztof DomańskiLooks good. One comment needs to be changed. Incorrect short description in doc comment of the
getFunction()
method.Comment #4
TR CreditAttribution: TR commentedThanks for catching that. There was a lot of cut-and-paste and I missed changing that.
Comment #5
Krzysztof DomańskiComment #6
TR CreditAttribution: TR commentedBump.
Comment #7
TR CreditAttribution: TR commentedPatch still applies and still passes tests, but there was an "unused use statement" so here's a new patch that removes that use statement.
Comment #8
pifagorComment #10
pifagor