drewm / morse

A feature detection library for PHP code that needs to run in multiple different environments
MIT License
160 stars 4 forks source link

Feature detection by constants to allow some autocompletion in IDE's #15

Closed nlzet closed 9 years ago

nlzet commented 9 years ago

I added all current available feature detections as a constant on the Morse class. This allows an IDE to auto complete to easily find a feature. As this library will grow something like this seems to be necessary. This will allow:

if (Morse::featureExists(Morse::FEATURE_FILE_FINFO)) {
    ...
}

I also added a test class which will check all available feature detections to ensure a constant with the correct name is available and has the correct value

drewm commented 9 years ago

Thanks for this. Forgive me being slow, as I've never used an IDE - all this is to allow for IDE autocomplete? So you can find a detection name without having to look at the docs?

I'm not opposed to the idea, but I wonder if there's another way to achieve the same end result. My main reservation is that the Morse class now needs to have knowledge of each of the detections in the Feature classes. It breaks the separation between the two.

So I think we should explore ideas that either:

  1. Enable declaration of the constants within the Feature class, maintaining separation of concerns, or
  2. Enable IDE autocompletion in some other way that achieves the same

Any ideas?

nlzet commented 9 years ago

No problem. Well, there are a lot of directions to go, but here are some thoughts i considered:

  1. Constants in each feature class are possible, but wouldn't help so much for auto completion since you have to know in which class you have to search.
    • For example with few knowledge of this library I wouldn't know to check for the exec function within the System class
  2. Use traits to include all methods in one class but keep all logic separated (in traits).
    • I think this would work great, but this might be a problem since traits are only supported since PHP 5.4
  3. Put all test methods in one class
    • You can auto complete everything but off coarse you will lose the separation of your logic. This also probably means you have to create a new major version.
  4. Using magic methods with __call on the morse class, and using PhpDoc's @method to reveal all features.
    • I believe this would work, but even tough this seems like a best of both worlds, I believe that isn't a neat solution, since we are creating/encouraging methods that don't really exist.
  5. My current solution
    • All constants in one class, and use them in the existing feature function, all auto completion can be done from within one class.

I'm definitely not saying my current solution is the best, but like i t is now, you have to know the correct string like system/exec, that doesn't seem right. IMO it would even would be better if you just have to call one method like Morse::featureExistsCacheApc() or maybe something like Morse::getCache()->existsApc(). (Altough the last proposal raises the same issue that I mentioned in 1.)

drewm commented 9 years ago

I feel a bit like this is the tail wagging the dog. Refactoring a loosely coupled design into a tightly coupled one just to enable IDE autocompletion seems like too much.

I don't think you're wrong in that this could be needed, but I'm also not convinced of it.

nlzet commented 9 years ago

Ok no problem, you may then close this PR.

drewm commented 9 years ago

I appreciate your contribution - thank you.