Replace interfaces with modified abstract classes

If we do this, it will mean that people have to extend our AbstractQuizQuestion in order to make a quiz question type. I don't think that's so bad, since there's a lot of logic in AbstractQuizQuestion that we rely on and would be difficult for any newcomer to do in a new class.

So I'm comfortable with using only abstract classes and not interfaces. When someone wants to make a new question type, we made need to work with them to make the abstract class more accessible to them, or even to make a new interface, but we can do that when/if necessary.

To stay completely OO, we need the interfaces. I have had reports from users (such as PanDa777) that they are using the interfaces and not the abstract classes, and we need the interface to enforce conformance to the expected methods.

So my vote would be to keep the interfaces, even though falcon is correct: none of our internal classes use the interface directly.

By the way, I think falcon is right about documentation of the abstract class. I think it should be well documented so that it is easier for new developers to read the abstract class and then write an extension.

Matt

I agree with mbutcher here on retaining them, but here's something kinda like a middle ground. I've only checked a few of the quiz questions, so I don't know that this would ACTUALLY work, but...sounds like what we should be doing is making the abstract parents implement the interfaces directly. In practice, this has exactly the same effect as putting abstract methods declarations into the parent classes (in fact, redeclaring methods as abstract when the interface already defines them causes a fatal php error), inasmuch as it'll require concrete extensions to implement anything the parent doesn't. Plus, using interfaces means that method signatures must be exactly as specified in the interface; abstract classes aren't so strict.

Anyway, if we do that, then people can freely use just the interfaces, or they can extend the parents and not have to worry about implementing the interfaces directly. So the interfaces kinda fade away into the background, while we still get all their warm and fuzzy goodness.

Well we should definitely have the abstract classes implement the interfaces. I thought they did that already. People who choose to extend the abstract classes don't have to deal with interfaces. That's well and good.

I'm okay with removing the interfaces for the time being. We can always put them back when someone comes along who wants to write a question type without using the abstract class.

But I'm fine with keeping them too. And if we do, I propose that for type hinting we use the interface instead of the abstract class. We should resolve this question before the type hinting gets added, #560274: Add class type hinting to all our OO code, and array type hinting where valuable.

/me points to #3, where mbutcher mentioned PanDa777 using just the interfaces, not the abstract classes :)

And absolutely +1 for type hinting. You can type hint strings and ints, too, as appropriate. Not that php's loose typing isn't great sometimes, but some of the biggest rabbit holes in Panels have come from php silently type-switching some function parameter...

No, the interfaces should exist. Maybe, I'm the only one who wants to be that way? Did mbutcher say his 'no' just because of my report (I changed my nickname recently, sorry)?. If it's so, I don't think I should be a breakpoint for that.

When I wanted to implement the new question type, I looked to AbstractQuizQuestionResponse, and I didn't liked it (I can't formulate, why). But I need the base for that, and the interface was perfect --- just because it is a good foolproof design.

And what is the common practice for Drupal OO?

Dmitry, is there anything you can't do by extending the abstract class that you can do by implementing the interface?

If there's something you don't like about AbstractQuizQuestionResponse, maybe we can simply fix that.

I don't believe there are common practices for Drupal OO yet. I think Quiz is on the frontier here. We could look to something like the Zend Framework for PHP OO practices. That has few interfaces and makes use mostly of inheritance. Interfaces are important in statically typed languages for creating terse contracts, but not so important in PHP.

If we don't have a specific reason to use interfaces besides some OO orthodoxy, then I agree with falcon that this project would be better off with them. (And of course, removing the ones we have wouldn't mean some ban on them forever. We can always add interfaces at points where they're justified.)

falcon, I agree with you about the OO behavior. As we discussed in IRC, I do find it confusing and "less OO" to have two methods in the abstract class for the hook functionality (e.g. save() and saveNode(). We should either make them methods that work as part of the class hierarchy or keep them in quiz_question.module, out of the class hierarchy.

I just committed changes the go with the former. Some of the names are different but to help people find the hook they're looking for, I put the hooks in the comments. I think this is fair as the QuizQuestion class is an *abstraction*. The critical methods for QuizQuestion classes (and thus an interface if we want one) are:

save() called by hook_save
delete() called by hook_delete
getNodeView() called by hook_view
getNodeProperties() called by hook_load

I committed these changes in three separate commits to make the edits easy to see.

While I was in there I noticed an inconsistency in how the relationships are handled. Sometimes the quiz_node_relationship table is modified in the QuizQuestion class and sometimes in quiz_question.module. We should keep it in one place or the other. I propose that it not be in the class as it's not really a concern of the question. The quiz_question.module can handle it. Also, it should not be available for child question types to override.