I'm writing a handler for download/import of data. After inserting data to the database, several import jobs can be called, but usually it is just one. So there are two methods to get the import job(s), that can be overridden from the abstract base class, one being abstract, the other one using the first one.
Please have a look at my code and doc and tell me if you think this design is okay or what practice may be best. I'm particular irritated by my design to have the one writing the subclass having to implement getImportJob()
just to throw an UnsupportedOperationException
, it somehow came up naturally. But is the bold marked part okay or am I breaking some conventions with that?
/**
* Returns the name and parameters of the import job that may be called
* after the data has been added to the database in
* {@link #insertDataIntoDatabase(Map)}.<br>
* If the import job is called or not in the aforementioned method is
* determined by {@link #isExecutingImports()}. If there is no import job to
* execute ever, this method should return <code>null</code>. If there are
* several import jobs to be done, this method should throw a
* {@link UnsupportedOperationException} and {@link #getImportJobList()}
* should be overridden to return the names and parameters of the import
* jobs in the order they should be executed.
*
* @return the name of the single import job that can be executed or
* <code>null</code> if none can ever be executed.
* @throws UnsupportedOperationException
* If this method was called when there are several import jobs.
*
* @see #getImportJobList()
*/
protected abstract String getImportJob();
/**
* Returns a list of names and parameters of import jobs that may be called
* after the data has been added to the database in
* {@link #insertDataIntoDatabase(Map)}.<br>
* The default implementation calls {@link #getImportJob()} and returns an
* empty or a singleton list depending on <code>getImportJob()</code> having
* returned <code>null</code> or a concrete object. When this method is
* overridden to provide several import jobs to be done,
* <code>getImportJob()</code> should also be overriden, according to its
* documentation.
*
* @return a list of names and parameters of import jobs
* @throws UnsupportedOperationException
* If <code>getImportJob()</code> was overridden to throw an
* <code>UnsupportedOperationException</code> without this
* method being overridden accordingly.
*
* @see #getImportJob()
*/
public List<String> getImportJobList() {
String singleImport = getImportJob();
if (singleImport == null)
return Collections.emptyList();
return Collections.singletonList(singleImport);
}
It just occured to me that I can make both methods abstract and provide two interfaces, one for single and one for multi use, each one leaving one method abstract and implementing the other one with default according to the contract... That may be a misuse of the default concept, but I think it would serve my purpose. My question still stands.
EDIT: Thanks to the answers I changed the base class to an interface without the single-Job method and added an inheriting interface with the single-Job method and a default for the Job-List method.
2 Answers 2
Academic: if part of your interface does not need to be implemented to satisfy your interface requirements, then your interface is poorly defined for your use case. What you really have is a different interface.
So: No, you cannot throw an UnsupportedOperationException. The exception should not even exist. "Unsupported Operations" and "Not Implemented Methods" are clearly communicated by virtue of the fact that I did not code the interface to support them. Such things are bubble gum and duct tape over poor designs.
Pragmatic: Implementing only part of an interface rather than the whole one is definitely a code smell and introduces the risk that a consumer of your implementation will experience behavior that should not be expected from your interface;use at your own peril.
One should design their systems as if all programmers are psychotic ax murderers who know where the designer lives. You would not be the first person to do such a thing, and anyone with Win32 experience would likely give you a sigh, an eyeroll, and some muttered epithets under their breath. Then they'd move on. You'd probably be safe.
-
What is the proper metaphor, then, for indicating that the consumer of your type or interface is responsible for the implementation?Robert Harvey– Robert Harvey2016年12月12日 17:24:35 +00:00Commented Dec 12, 2016 at 17:24
-
You are correct,
UnsupportedOperationException
should not exist and the fact that the JFC documents that it may be thrown by implementations ofCollection
is an aberration.user22815– user228152016年12月12日 17:26:26 +00:00Commented Dec 12, 2016 at 17:26 -
@RobertHarvey implementers are always responsible for the implementation, so I'm not sure that anything specific needs to be communicated. If the designer can anticipate that implementers will likely break the interface to get specific jobs done, then the designer should either provide the interface they anticipate or explicitly exclude it in the interests of keeping from communicating that feature from the interface.K. Alan Bates– K. Alan Bates2016年12月12日 17:28:15 +00:00Commented Dec 12, 2016 at 17:28
-
1@RoberHarvey
re:metaphor
...interface routes that support theNotImplementedException
andUnsupportedOperationException
are conceptually similar to nullable data columns represented in corollary programmatic interfaces. It should be taken as a matter of course that "all nullable data fields are design disappointments"; so too it follows for programmatic interfaces.K. Alan Bates– K. Alan Bates2016年12月12日 17:33:29 +00:00Commented Dec 12, 2016 at 17:33 -
I'll rephrase my question. In a type (let's say for this discussion's purposes that it is an abstract type), how do you communicate that there are methods which that be implemented, but for which there is no corresponding implementation in the abstract class? Do you simply let the abstract class derive from an interface containing the required methods to be implemented by the consumer of the abstract class?Robert Harvey– Robert Harvey2016年12月12日 17:44:32 +00:00Commented Dec 12, 2016 at 17:44
Is expecting the API user to implement an UnsupportedOperationException okay?
I would say not. You're likely contravening the Liskov substitution principle. Quoting:
No new exceptions should be thrown by methods of the subtype, except where those exceptions are themselves subtypes of exceptions thrown by the methods of the supertype.
so unless your base method is declared as throwing an UnsupportedOperationException
(or a superclass), that behaviour would be unexpected and would contravene the principle of least astonishment.
-
Since its a runtime exception, I thought documenting it would be enough :-/GreenThor– GreenThor2016年12月09日 16:23:34 +00:00Commented Dec 9, 2016 at 16:23
-
5I suspect I would only read the documentation once it's thrown that exception :-)Brian Agnew– Brian Agnew2016年12月09日 16:24:25 +00:00Commented Dec 9, 2016 at 16:24
-
@GreenThor and It should be. Long life to the unchecked Exceptions. But that's not the subject here :-)Laiv– Laiv2016年12月09日 19:22:24 +00:00Commented Dec 9, 2016 at 19:22
-
1Might also want to note that throwing an
UOE
suggests the interface is too broad and should be segregatedDioxin– Dioxin2016年12月09日 23:12:18 +00:00Commented Dec 9, 2016 at 23:12
Explore related questions
See similar questions with these tags.
getImportJobList()
that either returns null or a list with a single element doesn't make much sense to me.getImportJob()
. But if there are several import jobs, he is expected to overridegetImportJobList()
as well.UOE
is the pinnacle of code smells, and should only be used as a last resort. If a type does not support a behavior, it should not be forced to implement said behavior (or you violate ISP). If your type implements the interface and throws anUOE
, it's violating LSP. If you're forced to throwUOE
, the contract of the interface you are implementing is too broad for the type that is implementing it.