Unable to have an Abstract Data Object #2856

Closed
oilee80 opened this Issue Feb 17, 2014 · 5 comments

Comments

Projects
None yet
6 participants
Contributor

oilee80 commented Feb 17, 2014

Due to how the DataObject inheritance works it is not possible to have an abstract DataObject. The use case for this would be:

abstract class MediaLinkRelationship extends DataObject {
    private static $db = array('Title' => 'Varchar','AltName' => 'Varchar',.....);
    private static $has_one = array('MediaItem' => 'MediaItem');
}
class PageMediaLinkRelationship extends MediaLinkRelationship {
    private static $has_one = array('Parent' => 'Page');
}
class AnotherDataObjectMediaLinkRelationship extends MediaLinkRelationship {
    private static $has_one = array('Parent' => 'AnotherDataObject');
}

The error occurs during build as it is trying to instantiate the abstract class (MediaItemRelationship) during DatabaseAdmin::doBuild().

Contributor

tractorcow commented Feb 21, 2014

The problem is that user Extensions rely on instances, and the 'augmentDatabase' extension hook needs to be available to each discrete class at the point of DB generation. Not allowing DataObject classes to be abstract is one of the concessions that was made in favour of this functionality.

For the sake of pages, if you don't want a base class to be directly instantiated in the CMS you can at least use the hide_ancestor config. Other than this, what reason do you need abstract DataObject classes for? Is your problem not one that could be solved (for instance) using interfaces?

@simonwelsh simonwelsh added the 3.1 label Mar 15, 2014

@wilr wilr closed this Nov 15, 2014

Contributor

patricknelson commented Aug 16, 2016

But can't you still use reflection to ensure that a particular class isn't abstract prior to attempting to instantiate it? Why must it be instantiated? After all, it's the abstract class' child class which should be the target here, upon which the ->augmentDatabase() method could be called, for example. An important point to an abstract class is to ensure it's not instantiated (along with defining a good baseline API).

Due to the complexity of my code base (and the need to reuse code throughout the site), I'm finding more and more that I need to have the ability to setup an abstract class which itself isn't intended for instantiation, but can define a contract/API and consolidate some functionality at the same time.

Owner

dhensby commented Aug 16, 2016

I think the main issue is that the people who want abstract DataObjects haven't volunteered the effort to make it a reality. In the scheme of things, this isn't a priority for the core team.

Contributor

patricknelson commented Aug 17, 2016

I concur @dhensby, especially given the potential effort involved. What branch would a PR for this sort of change go into?

Contributor

tractorcow commented Aug 17, 2016

It would need to be master, since any existing code could rely on singleton($class) not to crash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment