Bug 180562 - [api] Classes should not implement interfaces just to bring constants into namespace
Summary: [api] Classes should not implement interfaces just to bring constants into na...
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 1.0.1   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 2.0   Edit
Assignee: David McKnight CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords: api
Depends on: 177297 178020 178566 183165
Blocks: 226364 226365 261486
  Show dependency tree
 
Reported: 2007-04-02 16:55 EDT by Martin Oberhuber CLA
Modified: 2009-01-19 06:19 EST (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Oberhuber CLA 2007-04-02 16:55:35 EDT
We consider it mis-use to have a class implement an interface for the sole purpose of bringing constants from that interface into the namespace of the class:

  public class Foo implements IFooConstants {
      public int bar() { return BAR_CONST; }
  }

instead, the constants should always be referenced directly:

  public class Foo {
      public int bar() { return IFooConstants.BAR_CONST; }
  }

bringing constants into the namespace of a class blows up Javadoc unnecessarily, and may bring constants which are meant to be "internal" into public API space when this is not desired.
Current examples of "constant" interfaces being implemented in RSE include:

IClientServerConstants
IDataStoreConstants
IHostSearchConstants
IRSEDOMConstants
IRSEUserIdConstants
IServiceConstants
ISystemContextMenuConstants
ISystemFilterSavePolicies
ISystemFilterConstants
ISystemIconConstants
ISystemPreferencesConstants
ISystemProcessPropertyConstants
ISystemProcessRemoteConstants
ISystemPropertyConstants
ISystemRemoteEditConstants
ISystemSearchConstants
ISystemTextEditorConstants
ISystemThemeConstants
ITarConstants
IUniversalDataStoreConstants
IUniversalProcessDataStoreConstants

And from import/export and user actions, there are:
IRemoteImportExportConstants
ISystemCompileXMLConstants
ISystemUDAConstants
Comment 1 Martin Oberhuber CLA 2007-04-02 17:21:37 EDT
There might be few exceptions to the general rule of NOT implementing "constant" interfaces: 
* when the number of constants is small, AND
* the class which implements them uses most (all) of the constants in API
  e.g. in order to get/set certain properties.

Classes MUST NOT implement constant interfaces which contain types that are implementation details - see bug 180568 for examples.

In order to get rid of the constant access, the "implements" clause must be removed from deriving classes; this will bring up JDT Errors. Then, all of these errors need to be navigated (works by keyboard shortcut) and the original interface name must be added as qualifier in front of the constant. I have, unfortunately, not found a way of doing this with less manual work.

Since it is considerable work, I propose separating it:
 - DaveD to do IRSEDOMConstants, IRSEUserIdConstants, ISystemFilter*
 - DaveM to do *DataStore*, *Property*
 - Kushal to do IHostSearch*, ISystemSearch*, ITar*, *Process*
Comment 2 David Dykstal CLA 2007-04-02 20:27:08 EDT
>  - DaveD to do IRSEDOMConstants, IRSEUserIdConstants, ISystemFilter*
Done.
Comment 3 Kushal Munir CLA 2007-04-03 05:01:14 EDT
>  - Kushal to do IHostSearch*, ISystemSearch*, ITar*, *Process*
Done.
Comment 4 David McKnight CLA 2007-04-03 10:42:07 EDT
-DaveM to do *DataStore*, *Property*
Done.

Is there anything left?
Comment 5 Martin Oberhuber CLA 2007-04-03 10:47:41 EDT
The following seem to be still open, if I'm not mistaken:

IClientServerConstants, IServiceConstants - DaveM
ISystemContextMenuConstants, ISystemIconConstants - DaveM

ISystemPreferencesConstants, ISystemThemeConstants - DaveD
ISystemRemoteEditConstants, ISystemTextEditorConstants - Kushal

Comment 6 Kushal Munir CLA 2007-04-03 11:31:41 EDT
> ISystemRemoteEditConstants, ISystemTextEditorConstants - Kushal
Done.
Comment 7 David McKnight CLA 2007-04-03 12:31:23 EDT
-IClientServerConstants, IServiceConstants, ISystemContextMenuConstants, ISystemIconConstants, ISystemMessages

done.
Comment 8 Martin Oberhuber CLA 2007-04-04 06:52:05 EDT
Also found ISystemOutputRemoteTypes, checking in this and DaveD's ones:
  [180562][api] dont implement ISystemOutputRemoteTypes
  [180562][api] dont implement ISystemPreferencesConstants
  [180562][api] dont implement ISystemThemeConstants
  [180562][api] dont implement IRemoteImportExportConstants
  [180562][api] dont implement ISystemCompileXMLConstants
  [180562][api] dont implement ISystemUDAConstants
bringing the full list to:

IClientServerConstants
IDataStoreConstants
IHostSearchConstants
IRemoteImportExportConstants
IRSEDOMConstants
IRSEUserIdConstants
IServiceConstants
ISystemCompileXMLConstants
ISystemContextMenuConstants
ISystemFilterSavePolicies
ISystemFilterConstants
ISystemIconConstants
ISystemOutputRemoteTypes
ISystemPreferencesConstants
ISystemProcessPropertyConstants
ISystemProcessRemoteConstants
ISystemPropertyConstants
ISystemRemoteEditConstants
ISystemSearchConstants
ISystemTextEditorConstants
ISystemThemeConstants
ISystemUDAConstants
ITarConstants
IUniversalDataStoreConstants
IUniversalProcessDataStoreConstants

Marking this fixed now.

Migration docs for users: If you migrate code to RSE 2.0, 
 - and error markers indicate that UPPERCASE constants can not be found,
 - and your class is extending an RSE base class,
it is likely that you need to prefix the UPPERCASE constant with one of the
constant holding interfaces mentioned above. Best use a text search to find
where the constant is declared (search workspace for UPPERCASE constant), and prefix the usage with the corresponding interface found.
Comment 9 Martin Oberhuber CLA 2008-08-13 13:17:42 EDT
[target cleanup] 2.0 M6 was the original target milestone for this bug