parse.com-php-library icon indicating copy to clipboard operation
parse.com-php-library copied to clipboard

Reason for inclusion of all files?

Open scarstens opened this issue 10 years ago • 2 comments

https://github.com/apotropaic/parse.com-php-library/blob/master/parse.php#L2

  1. These are class files, why are we not using "include_once" here?
  2. Is there a reason why you are including all files? Its very inefficient to load everything they way you do. All of the extension classes should instead include the parent class, and/or any other dependencies.

Example:

  • parse.php (should have no additional include files... except for Config, though I'm not thrilled about the use of the config class in general and think the config params should be part of the class instantiation.
  • parseObject.php (should include_once the parse.php file) Result: I can include parseObject.php, it will load the dependency class, and I can then work with objects, without loading all the other classes I'm not using.

If there is not some reason for this, I'll include the fixes to this in my next pull request.

scarstens avatar Apr 29 '14 07:04 scarstens

I actually left off working on an autoloading solution, so the include file will work for now, but won't be necessary when I get that new version up and going.

As for config files, I've gone back and forth on this. V1 did loading throw class instantiation: https://github.com/apotropaic/parse.com-php-library/blob/parse.com-php-library_v1/parse.php

And there were a couple reasons why I switch to a config file model in v2. But one developer got me thinking about this a few months back and I proposed this idea, but never really landed on it: https://github.com/apotropaic/parse.com-php-library/commit/eba20179905de092c5054a903822c2f808ba560b

With v2 I wanted to make the entire library more flexible to use as much or a little as you wanted of it. Which ironically I botched as you've pointed out with all my include statements haha!

One reason I went to the config file model was it seemed to make the post sense to achieve this line here: https://github.com/apotropaic/parse.com-php-library/blob/master/parseObject.php#L15

So, I can call just $object = new ParseObject(); without having to include the app id and keys every single time. Maybe there is a different or better way of doing this. I'm totally open to feedback and change.

Like I said with the new version, I'm looking at adopting modern standards, and make this entire library a composer package. Config files are handled much different there and should solve problems there.

andrewscofield avatar Apr 29 '14 07:04 andrewscofield

I like the new config idea better then the existing, however it still requires that you "create a file and place it in the library". I didn't develop a way around the config file yet, so I don't have a "solution" to provide you with. It is convenient to not have to pass all that info, as you mention. But what if I wanted to connect to 2 different Parse.com instances? I think its impossible with the current setup, but you can only have 1 config. I'll have to give the config concept some more thought.

I did however reverse the file inclusion to follow the "class dependency" method, and added it to my pull request: https://github.com/apotropaic/parse.com-php-library/pull/149

Thanks for the responses, its been refreshing to meet another active developer on Github... instead of the build and abandon projects I usually run across.

scarstens avatar Apr 29 '14 07:04 scarstens