influxdb-php
influxdb-php copied to clipboard
influxdata
Better exception class naming
All exception classes of this library are named Exception.
If we import one of them, the code looks like:
try {
$stats = $this->get('influx.wrapper')->getLastServerStat($server->getCompleteName());
} catch (Exception $e) {
}
Hard to see at the first move the exception is talking about query, database or whatever.
Having QueryException for example would be a good start.
What do you think?
The Exception part of the name can even be considered useless :
- http://mnapoli.fr/approaching-coding-style-rationally/
- https://vimeo.com/album/2661665/video/74316116
But that would need creating more exception classes, because you can't really rename DatabaseException to just Exception\Database, but to CannotCreateDatabase.
Hard to see at the first move the exception is talking about query, database or whatever.
Of course, you can still alias the Exception to DatabaseException, but this is quite cumbersome, so :+1: for this suggestion, and if you can, consider even dropping the Exception suffix.
The
Exceptionpart of the name can even be considered useless :
:-1: for that. I think it easier to see what we are using with Interface or Exception suffixes.
It's the way used by many libraries/framework. Symfony for example follow this method.
In any way, we both agree for the Exception custom naming.
In any way, we both agree for the Exception custom naming.
Indeed, it must be changed.
It's the way used by many libraries/framework. Symfony for example follow this method.
That's a fallacy.
I think it easier to see what we are using with Interface or Exception suffixes.
For interface, I'd argue that you shouldn't care if you are type hinting against a class or an interface, it's not the concern of the consuming code, it's an implementation detail.
For exceptions, I'll just parrot the video : "when do you not know it's an exception?". Indeed, the catch and throw keywords are already sufficient to ensure that.
It's always better to type hint an interface if possible.
Having the suffix make code reading easier IMHO.
Same for exception. You're maybe right for the try catch but not for imports list for example. And yes, I'm use to read it. ;-)
Sullivan SENECHAL
Le 18 juil. 2016 17:54, "Grégoire Paris" [email protected] a écrit :
In any way, we both agree for the Exception custom naming.
Indeed, it must be changed.
It's the way used by many libraries/framework. Symfony for example follow this method.
That's a fallacy https://yourlogicalfallacyis.com/bandwagon.
I think it easier to see what we are using with Interface or Exception suffixes.
For interface, I'd argue that you shouldn't care if you are type hinting against a class or an interface, it's not the concern of the consuming code, it's an implementation detail.
For exceptions, I'll just parrot the video : "when do you not know it's an exception?". Indeed, the catch and throw keywords are already sufficient to ensure that.
or interfaces, you shouldn't care, because whether you are type hinting against a class or an interface should be just an implementation detail.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/influxdata/influxdb-php/issues/54#issuecomment-233371816, or mute the thread https://github.com/notifications/unsubscribe-auth/ABnqNaV0PN6WZpZBVMmg-rIs1U3_MIZcks5qW6HJgaJpZM4JO0Hy .
imports list for example
use MyCompany\MyProject\Exception\InvalidArgumentException
You're right, having Exception twice makes it easier to read. But you know what would be even easier? This :
use MyCompany\MyProject\Exception\InvalidArgumentExceptionExceptionException
:trollface: :trollface: :trollface:
It's always better to type hint an interface if possible.
Yes, but it's not better to let that be the worry of the dependency writer consider this :
class Cache
If you depend on a Cache, and then you realize you have to create other implementations, you just rename your class to FileCache, create MemcachedCache, and create a Cache interface. Classes used to depend on Cache don't need to rename anything (provided the interface is in the same namespace as the implementations). I'm just repeating what I read in the article here :P This one, which is linked in it, goes as far as saying that no, it's not always better, especially if you have only one possible implementation.
You got some point indeed.
But I still think suffix and prefix are great for consistency. Different opinions, no more argument for me.
It's the way used by many libraries/framework. Symfony for example follow this method.
That's a fallacy.
Please don't do it again. We already discussed that. You think it's a fallacy? So ok just stop following PSR and write your code with tabs and alignment!
I never say this is the argument but it's a fact: Most of the libraries work like that and I don't think it's just for fun.
PSR is different IMO: it's a discussed and validated standard, not a copied habit. They might not do it for fun, but can you find one that says why, besides "they do it like that over here"?
To reword all this in a less troll-esque way, it's indeed right that many library do this. Beyond them, a good part do it just because other do them, and they want to apply the principle of least astonishment, or maybe fear something bad will happen if they don't. Some other might have found an actual reason for this but if this is the case, aren't you a bit curious about it? I think that this decision happens very early in the life of a developer, maybe too early, and often, young developers will mimic what they see without searching to know why it was done like that.
A quick research links to this question.
The second answer show that a reason for people to do that might be to avoid conflicts. It dates from a time when there was no namespaces. So this might be a relic on the past
The first one invokes the same reason you invoke: clarity, and the same argument is objected : "I thought this, but it will only ever be used in the context of class FileNotFound extends Exception, or when throwing and catching", and then another argument comes : "yes, but if you work with an IDE, the class will appear in your project explorer/code explorer, and may create confusion. With the "Exception" prefix, things are clearer in the long run IMO. " (he means suffix, actually). With the namespaces this is no longer valid either in my opinion.
This question shows a better argument : class names should be nouns, that's probably why it feels weird not to have this suffix.