1

We have an existing project that has grown very messy and confusing; our classes are huge and difficult to read.

Our boss wants us to break all of our methods out of the classes and into separate files (for maintainability), so that it would be something like this.

class Foo {

    public function bar()
    {
        include 'Foo/Bar.php';
    }

    public function baz()
    {
        include 'Foo/Baz.php';
    }
}
class Bar {

    public function foo()
    {
        include 'Bar/Foo.php';
    }
}

We have about 1000 methods that will be broken out into separate files and then included into their respective classes; so basically 1000 additional included files.

Breaking apart classes and putting their methods into separate files is not something that I would do, and I've looked around to see if anyone else is doing this but I'm not getting good search results, which makes me think this is not something many people are doing. But I could be wrong!

I've done some testing and of course this many includes will affect I/O, but how badly? Especially if using a cache?

Using this approach, what are your thoughts and concerns? Do you foresee any problems happening in regards to using $this?

Tim
  • 11
  • 1
  • 3
    This is positively insane. I... I just... really? *That's* the proposed solution? – Greg Burghardt May 25 '21 at 21:59
  • Which version of PHP are you using? – Greg Burghardt May 25 '21 at 22:03
  • Not an exact duplicate, but fixing this the right way would be covered in this question: [I've inherited 200K lines of spaghetti code — what now?](https://softwareengineering.stackexchange.com/q/155488/118878). – Greg Burghardt May 25 '21 at 22:10
  • @GregBurghardt I think it's 7.4 but I'm not sure. Haven't been able to look at the server yet. – Tim May 25 '21 at 22:31
  • You're basically fixing the problem that the classes are huge and difficult to read by making them huge and *impossible* to read because all the important stuff is splattered around a gazillion files. The fix is not to hide the code away but to … you know … fix the code. – Jörg W Mittag May 25 '21 at 22:52
  • Could you use [PHP Traits](https://www.php.net/manual/en/language.oop5.traits.php)? You could group similar methods in the same trait. It is still the same basic mess, but at least it would be a categorized mess that could lead to splitting these classes up later on. – Greg Burghardt May 25 '21 at 22:56
  • It's both an ok idea and a bad idea. Ok because you will be able to view all the functions of the class at once, since their contents are elsewhere, making the easier to understand. But its also a bad idea because each method of the class is not ment to be coded in isolation from the other methods. – Richard Bamford May 26 '21 at 12:26
  • If you put every method into its own file, that would be bad enough. But you are just putting the bodies of these methods into a file! That is absolute madness. Get rid of that boss. – gnasher729 May 26 '21 at 17:40
  • There's implied assumption that "our code is confusing and hard to work with because it's long" here, and I don't think it's correct. Length alone doesn't make code that hard to follow. If all the parts interact clearly, and you can follow along, it's no problem. I'd say the problem is more likely to be the number of variables, and the way they might be changing under your feet by external interactions, which makes it impossible to reason locally. If that's the case, merely splitting up the ball of mud between files won't actually help at all. – Alexander May 26 '21 at 19:07
  • Wouldn't this also ruin source control history? You'd have to know about the old file version and its location to go back and look at code changes over time. – Graham May 27 '21 at 12:53

1 Answers1

3

It seems like a super bad idea, but include does do this exact thing.

I would say its a very "procedural" way of thinking, you're assembling code like it's a single page which is going to run from top to bottom. Perhaps that kind of approach is how you ended up with your current problem?

Obviously its impossible to know without seeing the actual codebase, but maybe a middle ground approach might be acceptable.

Split the classes up, but group several related methods rather than putting each in its own file. That way you are working towards new smaller classes rather than towards what would seem to amount to many functions with global state, almost the definition of "spaghetti code."

Robert Harvey
  • 198,589
  • 55
  • 464
  • 673
Ewan
  • 70,664
  • 5
  • 76
  • 161