[Embedded JBoss Development] - Re: Embedded API Design Problems
by ALRubinger
"richard.opalka(a)jboss.com" wrote : Do you know golden rule: Design your API properly since beginning. Later there will be no will to rewrite it?
Absolutely. Which is precisely why I engaged the community with pointers on the jboss-development list and a "Getting Started" Wiki months before even the first release. Part of this is "planning for change", so "Alpha" denotes we can make these types of adjustments before locking down.
I'll look into the rest later on, but I doubt you'll be able to convince me that it's a bad idea to declare an unchecked exception . No bad can come of it, and it's self documenting.
Actually there's a few generics issues I have patches for in jboss-bootstrap anyway, but didn't merge them in yet as to not slow down this release.
S,
ALR
View the original post : http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4268855#4268855
Reply to the post : http://www.jboss.org/index.html?module=bb&op=posting&mode=reply&p=4268855
16 years, 4 months
[Embedded JBoss Development] - Re: Embedded API Design Problems
by david.lloyd＠jboss.com
"richard.opalka(a)jboss.com" wrote : Please take it as feedback and not hassling ;)
|
| INCONSISTENT METHOD NAMES
|
| * ISSUE 1: Many Embedded project classes/intefaces follow BEAN naming convention but there are some exceptions.
|
| * RULE 1: API should be consistent. Once you're following BEAN naming convention, whole API must apply to this rule.
|
I buy this, even with builder-pattern-like things. Mutators should have "set" in front of them, even if they return "this" for use in method call chaining.
BTW, rather than using a generic type paramter representing "this class", you should just leverage covariant return types and have each subtype override the return type with its more specific type. With the generic parameter this is being done anyway; the difference is, now users often have to deal with the onerous and obnoxious "this" type parameter. Any time you have "Foo<T extends Foo<T>>" that should be a warning to proceed with caution.
"richard.opalka(a)jboss.com" wrote :
| DO NOT DECLARE TO THROW RUNTIME EXCEPTIONS
| ...
| * RULE 2.5: Runtime exceptions indicate programmer errors. The best practice is to don't declare them in method signature.
|
Couldn't disagree more. Putting all possible/expected unchecked exception types in the throws clause gives users a sense of what to expect; also it can hint to IDEs that you might want to add a catch clause for it. And it does no real harm.
"richard.opalka(a)jboss.com" wrote :
| * shouldn't be declared to be thrown from API if it is assignable from java.lang.RuntimeException and represents programmer error
|
Again, disagree.
View the original post : http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4268850#4268850
Reply to the post : http://www.jboss.org/index.html?module=bb&op=posting&mode=reply&p=4268850
16 years, 4 months
[Embedded JBoss Development] - Re: Embedded API Design Problems
by richard.opalka＠jboss.com
"ALRubinger" wrote :
| Thanks for the feedback, Richard. We're at Alpha-level to encourage this kind of discussion.
|
Do you know golden rule: Design your API properly since beginning. Later there will be no will to rewrite it?
"ALRubinger" wrote :
| I'm not strictly following bean conventions. For the method chaining signatures, I considered Fowler's note on command query separation as detailed in http://martinfowler.com/dslwip/MethodChaining.html.
|
Thanks, I will have a look. Maybe I'll change my mind ;)
"ALRubinger" wrote :
| I'm a big fan of declared unchecked exceptions; they more completely document the API.
|
If you mean declaring it using @throws clause, I'm fine with you otherwise let me disagree.
See RULE 41 from Effective Java that says:
In case programmer that is using your API cannot handle thrown exception,
it's more proper to declare it as unchecked exception.
"ALRubinger" wrote :
| I don't care if the exception thrown is in a different package (eg Rule 2.2 "FileOutputStream.write(byte[]) throws ArrayIndexOutOfBoundsException"). Rule 2.3, consistency, is a design goal and any places that do not follow this need to be ironed out; which did you mean in particular?
|
See RULE 43 (Exception translation) from Effective Java.
"ALRubinger" wrote :
| In order to act upon rule 2.4, forcing the user to deal w/ checked exceptions, I'll need some examples you believe need attention. I don't agree with Rule 2.5.
|
What about RULE 40 from Effective Java ;)
(Regarding examples that need your attention, I provided one. Do you think it's not relevant one?)
"ALRubinger" wrote :
| From where are you deriving these rules BTW?
|
>From the same source like you Effective Java (my notes)
"ALRubinger" wrote :
| I don't consider Java Platform exceptions to be unnecessarily low-level; in fact Bloch encourages their use in validating preconditions/postconditions (I'd make a reference here but don't have my book w/ me). These are well-known APIs which programmers are likely to know already. Why wrap these in some Embedded-specific API? IllegalArgumentException and InstanciationException means exactly that.
|
This is again RULE 43 (Exception translation) from Effective Java.
"ALRubinger" wrote :
| I don't see how the generics issue you point out is an inconsistency. Also "other inconsistencies" doesn't help find action items. :)
|
I provided you examples of two classes from two different packages implementing the same feature.
Still can't see it? ;)
View the original post : http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4268834#4268834
Reply to the post : http://www.jboss.org/index.html?module=bb&op=posting&mode=reply&p=4268834
16 years, 4 months
[Embedded JBoss Development] - Re: Embedded API Design Problems
by ALRubinger
Thanks for the feedback, Richard. We're at Alpha-level to encourage this kind of discussion.
"richard.opalka(a)jboss.com" wrote : INCONSISTENT METHOD NAMES
| API should be consistent. Once you're following BEAN naming convention, whole API must apply to this rule.
I'm not strictly following bean conventions. For the method chaining signatures, I considered Fowler's note on command query separation as detailed in http://martinfowler.com/dslwip/MethodChaining.html.
"richard.opalka(a)jboss.com" wrote : DO NOT DECLARE TO THROW RUNTIME EXCEPTIONS
I'm a big fan of declared unchecked exceptions; they more completely document the API. I don't care if the exception thrown is in a different package (eg Rule 2.2 "FileOutputStream.write(byte[]) throws ArrayIndexOutOfBoundsException"). Rule 2.3, consistency, is a design goal and any places that do not follow this need to be ironed out; which did you mean in particular? In order to act upon rule 2.4, forcing the user to deal w/ checked exceptions, I'll need some examples you believe need attention. I don't agree with Rule 2.5.
>From where are you deriving these rules BTW?
"richard.opalka(a)jboss.com" wrote : INCONSISTENT ENUM VALUES
"PRE_INIT" seemed more descriptive to me; it's fired *before* initialization is called. Your suggestion to "INITIALIZING" isn't accurate as the server is not in the process of actually initializing; perhaps we should go to "PRE_INITILIZATION" or similar?
"richard.opalka(a)jboss.com" wrote : INCONSISTENT API ABSTRACTION
|
| Instead of throwing low level exception API designer should create exception that is consistent with API abstraction.
| In many cases such main abstraction exception don't need to be specified in throws clausule (just throw it).
I don't consider Java Platform exceptions to be unnecessarily low-level; in fact Bloch encourages their use in validating preconditions/postconditions (I'd make a reference here but don't have my book w/ me). These are well-known APIs which programmers are likely to know already. Why wrap these in some Embedded-specific API? IllegalArgumentException and InstanciationException means exactly that.
"richard.opalka(a)jboss.com" wrote : OTHER CODE INCONSISTENCIES
I don't see how the generics issue you point out is an inconsistency. Also "other inconsistencies" doesn't help find action items. :)
S,
ALR
View the original post : http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4268821#4268821
Reply to the post : http://www.jboss.org/index.html?module=bb&op=posting&mode=reply&p=4268821
16 years, 4 months
[JBoss AS Development] - Re: Graceful Shutdown
by bstansberry＠jboss.com
When I used "ad-hoc" about the connector start/stop, my meaning was the same as yours -- relying on the JMX notification is not good, as it's completely outside of how all other ordering is done, can break at any time etc.
The more I think about it, the more I like combining the per-webapp thingys into a single thingy that also handles the connector start/stop. It becomes:
GracefulStart:
Start the connector.
Graceful stop:
Coordinate with the deployed webapps and their session managers to go through all the details of draining sessions and ensuring in-flight requests have completed.
Stop the connector.
My experiment is going to follow this approach until it proves unworkable.
Re: install callbacks if the "central management bean" is deployed last; I have a vague recollection that I thought that wouldn't work for some other usage, but it did. But I could be misremembering, or maybe there was some weird fluke that made it work.
If that's a problem, we can just divide the "central management bean" into two parts:
Registry: deployed in bootstrap. Gets all the install callbacks
Invocation: deployed at the end. Has registry injected. Invokes on the registered items.
View the original post : http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4268814#4268814
Reply to the post : http://www.jboss.org/index.html?module=bb&op=posting&mode=reply&p=4268814
16 years, 4 months
[Embedded JBoss Development] - Embedded API Design Problems
by richard.opalka＠jboss.com
Please take it as feedback and not hassling ;)
INCONSISTENT METHOD NAMES
* ISSUE 1: Many Embedded project classes/intefaces follow BEAN naming convention but there are some exceptions.
* RULE 1: API should be consistent. Once you're following BEAN naming convention, whole API must apply to this rule.
Example:
package org.jboss.bootstrap.api.as.config;
|
| public interface JBossASBasedServerConfig<T extends JBossASBasedServerConfig<T>> extends ServerConfig<T>
| {
| ...
| String getBindAddress();
| T bindAddress(String bindAddress); // this is WRONG, see RULE 1
| ...
| }
|
| package org.jboss.bootstrap.api.server;
|
| public interface Server<K extends Server<K, T>, T extends ServerConfig<T>>
| {
| ...
| T getConfiguration();
| void setConfiguration(T config); // this is CORRECT
| ...
| }
|
SUGGESTION: The best practice is to follow bean naming convention for setter/getter methods.
DO NOT DECLARE TO THROW RUNTIME EXCEPTIONS
* ISSUE 2: Many embedded project classes/interfaces declare to throw RuntimeExceptions, but only for some methods
* RULE 2.1: Declare exception to be thrown if its abstraction is consistent with class/API (correct: FileOutputStream.write(byte[]) throws IOException)
* RULE 2.2: Declare exception to be thrown if it's expected (wrong: FileOutputStream.write(byte[]) throws ArrayIndexOutOfBoundsException)
* RULE 2.3: Once API designer decides to declare to throw exception this decision must be consistent for whole API.
* RULE 2.4: Throwing checked exceptions force users of the API to catch or propagate them, consider this when designing API.
* RULE 2.5: Runtime exceptions indicate programmer errors. The best practice is to don't declare them in method signature.
Example:
package org.jboss.bootstrap.api.as.config;
|
| public interface JBossASBasedServerConfig<T extends JBossASBasedServerConfig<T>> extends ServerConfig<T>
| {
| ...
| URL getServerLibLocation();
| T serverLibLocation(URL serverLibLocation); // this is WRONG: see RULE 2.3
| T serverLibLocation(String serverLibLocation) throws IllegalArgumentException; // this is WRONG - see RULE 2.1, RULE 2.2, RULE 2.4 and RULE 2.5
| ...
| }
|
|
INCONSISTENT ENUM VALUES
* ISSUE 3: There are inconsistent enum values in embedded API.
* RULE 3: Always define consistent enums/constants to don't confuse users.
Example:
package org.jboss.bootstrap.api.lifecycle;
|
| public enum LifecycleState
| {
| INSTANCIATED,
| PRE_INIT, // this is WRONG, should be INITIALIZING to be consistent with other constants
| INITIALIZED,
| IDLE,
| STARTING,
| STARTED,
| STOPPING,
| STOPPED
| }
|
|
INCONSISTENT API ABSTRACTION
* ISSUE 4: There are low level exceptions (including runtime exceptions) declared to be thrown (checked exceptions) in some methods, in others not.
* RULE 4: Instead of throwing low level exception API designer should create exception that is consistent with API abstraction.
In many cases such main abstraction exception don't need to be specified in throws clausule (just throw it).
Example:
package org.jboss.bootstrap.api.factory;
|
| public class ServerFactory
| {
| ...
| public static Server<?, ?> createServer(final String implClassName)
| throws IllegalArgumentException, InstantiationException; // this is WRONG, see RULE 4
|
| public static Server<?, ?> createServer(final String implClassName, final ClassLoader cl)
| throws IllegalArgumentException, InstantiationException // this is WRONG, see RULE 4
| ...
| }
|
| public class ServerFactory
| {
| ...
| public static Server<?, ?> createServer(final String implClassName) // this is CORRECT
| {
| ...
| catch (InstantiationException ie)
| {
| throws BootstrapException(ie);
| }
| ...
| }
|
| public static Server<?, ?> createServer(final String implClassName, final ClassLoader cl) // this is CORRECT
| {
| ...
| catch (IllegalArgumentException ie)
| {
| throws BootstrapException(ie);
| }
| ...
| }
| }
|
SUGGESTION: Create abstract exception that best fulfills the API abstraction and consider if it will be thrown from the API or not.
* should be declared to be thrown from API if it is not assignable from java.lang.RuntimeException and user have to deal with it
* shouldn't be declared to be thrown from API if it is assignable from java.lang.RuntimeException and represents programmer error
Example:
* BootstrapException extends RuntimeException // defining API abstraction exception - it's Throwable getCause() returns the original exception that caused the failure
* LifecycleEventException extends BootstrapException // all exceptions defined in embedded API should extend main abstraction exception
OTHER CODE INCONSISTENCIES
* ISSUE 5: There are many inconsistencies in embedded API cross different components.
* RULE 5: Avoid API inconsistencies. They usually indicate wrong design or decomposition of the problem.
Example:
package org.jboss.bootstrap.api.mc.server;
|
| public interface MCBasedServer
| <
| K extends org.jboss.bootstrap.api.server.Server<K, T>,
| T extends org.jboss.bootstrap.api.config.ServerConfig<T>
| >
| extends org.jboss.bootstrap.api.server.Server<K, T>
| {
| org.jboss.kernel.Kernel getKernel();
| }
|
| but
|
| package org.jboss.bootstrap.api.as.server;
|
| public interface JBossASBasedServer
| <
| K extends JBossASBasedServer<K, T>, // this is WRONG: shouldn't it be K extends org.jboss.bootstrap.api.server.Server<K, T> ?
| T extends org.jboss.bootstrap.api.as.config.JBossASBasedServerConfig<T> // this is WRONG: should be T extends org.jboss.bootstrap.api.config.ServerConfig<T> ?
| >
| extends org.jboss.bootstrap.api.server.Server<K, T>
| {
| }
View the original post : http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4268805#4268805
Reply to the post : http://www.jboss.org/index.html?module=bb&op=posting&mode=reply&p=4268805
16 years, 4 months