Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Move the PHP classes from the DCA files into separate files #7649

Closed
leofeyer opened this issue Feb 20, 2015 · 16 comments
Closed

Move the PHP classes from the DCA files into separate files #7649

leofeyer opened this issue Feb 20, 2015 · 16 comments
Labels
Milestone

Comments

@leofeyer
Copy link
Member

What's the best way to accomplish this? My idea is to create a new subfolder called dcac (the additional "c" means "classes" or "callbacks") and to store the classes prefixed with Dca in it.

system
  modules
    core
      dca
        tl_member.php
        tl_user.php
        tl_news_archive.php
      dcac
        DcaMember.php
        DcaUser.php
        DcaNewsArchive.php

@contao/developers What do you think?

@leofeyer leofeyer added this to the 3.5.0 milestone Feb 20, 2015
@aschempp
Copy link
Member

Is there a short-time need to change that?

In Isotope we switched to a \Isotope\Backend namespace in version 2 (https://github.com/isotope/core/tree/master/system/modules/isotope/library/Isotope/Backend) and real classes with PSR-0, but that's obviously not BC.

@leofeyer
Copy link
Member Author

I don't think a BC break is necessary here. Let's just move the PHP classes to where they should have been from the beginning; it's an easy pick and it will give people the opportunity to get used to a new way of doing things, which we might enforce in a future Contao version.

@aschempp
Copy link
Member

it will give people the opportunity to get used to a new way of doing things, which we might enforce in a future Contao version.

I really disagree, because the new way should be to write PSR-0/4 classes in a namespace with CamelCase names, no dependency on a Backend class etc. Adding a dcac folder would simply be introducing another "bad practice" for classes.

@leofeyer
Copy link
Member Author

No, that's actually the new Contao 4 way. The "new thing" I was referring to is simply not to put PHP classes into DCA files :)

We have agreed to fix this in on of our last calls, so the question is actually now "if" but "how".

@Zeromax
Copy link

Zeromax commented Feb 21, 2015

@leofeyer I like the idea.

Just a question: will those files/classes be cached? Cause now they are cached or am I wrong?

@leofeyer leofeyer modified the milestones: 4.0.0, 3.5.0 Feb 22, 2015
@aschempp
Copy link
Member

We have discussed not to change this in Contao 3. Still we should get rid of the classes in DCA files, but we need to check how that can be backwards compatible.

@leofeyer
Copy link
Member Author

If we waited for Contao 5, we could break backwards compatibility.

@aschempp
Copy link
Member

True, or we use class_alias (which is very hacky)…

@BugBuster1701
Copy link
Contributor

And this is not possible?
If the directory "dcac" is defined in the autoload.php, it should also work backward compatible.

system
  modules
    core
      dca
        tl_member.php
        tl_user.php
        tl_news_archive.php
      dcac
        tl_member.php
        tl_user.php
        tl_news_archive.php

@leofeyer
Copy link
Member Author

Yes, it would be possible. And backwards compatible.

@leofeyer leofeyer modified the milestones: 3.5.0, 4.0.0 Feb 27, 2015
@leofeyer
Copy link
Member Author

@aschempp What about @BugBuster1701's proposal? Do we want to change it this way?

@Tastaturberuf
Copy link
Contributor

👍 for @BugBuster1701s solution

@Toflar
Copy link
Member

Toflar commented Mar 19, 2015

I'm clearly 👎 on this.

The only advantage this generates is that the internal cache file for the DCA gets smaller and to me this is simply waisted time and effort. Better invest that time on converting callbacks to events and have real PSR-0/4 paths for the listeners in a future version.

@aschempp
Copy link
Member

I'm with @Toflar here, it makes no sense to change something to a still-incorrect solution (because not using PSR-0/1/2/4). It's old stuff that should be fixed later once and for all.

The only good we could do is add a statement to each class saying people should not follow this bad example…

@fiedsch
Copy link

fiedsch commented Mar 19, 2015

I dislike @BugBuster1701 solution because when I read dcac I always think it has to be ACDC (SCNR).
To be serious: +1 for @Toflar and @aschempp especially with the the upcoming Contao 4 in mind

@leofeyer
Copy link
Member Author

Ok.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants