Bug 210109 - [api] FILE_TYPE_* constants should be in IFileService instead of IFileServiceConstants
Summary: [api] FILE_TYPE_* constants should be in IFileService instead of IFileService...
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 3.0   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 3.0 M4   Edit
Assignee: David McKnight CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks:
 
Reported: 2007-11-16 10:45 EST by Martin Oberhuber CLA
Modified: 2009-06-17 14:45 EDT (History)
0 users

See Also:


Attachments
patch with some docs for the file service constants (3.03 KB, patch)
2007-11-16 11:51 EST, David McKnight CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Oberhuber CLA 2007-11-16 10:45:11 EST
In RSE 3.0M3, new file IFileServiceConstants.java was added just to hold the
  FILE_TYPE_FILES_AND_FOLDERS
  FILE_TYPE_FILES
  FILE_TYPE_FOLDERS
constants.

I think that since these constants are very tightly coupled to the IFileService API, they should be defined in IFileService itself. IFileServiceConstants should be removed again. Also, Javadoc needs to be added to those constants.

Take IResource as an example of what the Javadoc and layout can look like. I'd like to see FILE_TYPE_FILES and FILE_TYPE_FOLDERS treated as bit masks, allowing for later extension with other possible types. FILE_TYPE_FILES_AND_FOLDERS is special in the sense that it does apply the name filter to the files, but always returns ALL the folders. That should also be documented.

Declaring the constants in IFileService has another advantage: Since AbstractFileService implements IFileService, the constants will be visible in the name space for file service implementors automatically, as it was in the TM 2.x release where they were declared in AbstractFileService. 

This means, that when the constants are declared in IFileService, existing old implementations will continue to work and we do not unnecessarily break API.
Comment 1 David McKnight CLA 2007-11-16 11:22:17 EST
Fair enough, for now I've moved the constants into IFileService.  I haven't changed anything to use bitmasks yet though.
Comment 2 Martin Oberhuber CLA 2007-11-16 11:26:35 EST
The bitmask thing is only for documentation anyways, because the current values (1 and 2) are valid bit masks already.

It's only in order to document that
  FILE_TYPE_FILES_AND_FOLDERS
is special an remains value 0, so client code needs to handle it specially;
for others, client code should be able to do bitmasking.
Comment 3 David McKnight CLA 2007-11-16 11:51:50 EST
Created attachment 83081 [details]
patch with some docs for the file service constants

I've attached a patch with some javadoc for the file service constants.  Is this the sort of thing you're looking for?  I kept FILE_TYPE_FILES_AND_FOLDERS as it's own value insteand fo combining FILE_TYPE_FILES and FILE_TYPE_FOLDERS since it's unique in that only files are filtered.  Right now, if a user does combine things our IFileService implementors are not handing the combinations.
Comment 4 Martin Oberhuber CLA 2007-11-16 12:02:19 EST
Docs look great, except for one thing:

You must not change the value of FILE_TYPE_FILES_AND_FOLDERS, because existing code will have coded against the old value (0) and will have that value inlined into its class files. Also, value 3 is effectively the bit mask of 1 | 2 so one could not see the special meaning of it:

Bit mask 1 means "give me files, filtered by name"
Bit mask 2 means "give me folders, filtered by name"
--> mask 3 MUST BE "give me folders and files, both filtered by name"

Therefore we must keep the special value 0 to mean "give me files filtered by name and folders unfiltered".

If we want to think ahead for what we want to do with other types (links, devices, special files, drives, mount points, junctions, named pipes, sockets, ...) we could discuss whether we want to define the meaning of value 0 as "Give me ANYTHING and only filter files by name" or similar.
Comment 5 David McKnight CLA 2007-11-16 12:16:30 EST
Okay, I've committed the code as suggested.