Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Creating DataHandle<Location> from FileLocation and BytesLocation #468

Closed
Daniel-Alievsky opened this issue Aug 6, 2023 · 2 comments
Closed

Comments

@Daniel-Alievsky
Copy link

Most popular situations, when we need DataHandle, are processing files and byte arrays, implemented in the classes FileHandle and BytesHandle. Unfortunately, the only correct way to create the necessary objects is using SciHava context technique:

context.getService(DataHandleService.class).create(location)

But initializing context sub-system requires time and special code. I would be very desirable to avoid Context class in these very simple situation.

I've found necessary solution as a pair of simple functions:

    @SuppressWarnings("rawtypes, unchecked")
    public static DataHandle<Location> getFileHandle(FileLocation fileLocation) {
        Objects.requireNonNull(fileLocation, "Null fileLocation");
        FileHandle fileHandle = new FileHandle();
        fileHandle.set(fileLocation);
        return (DataHandle) fileHandle;
    }

    @SuppressWarnings("rawtypes, unchecked")
    public static DataHandle<Location> getBytesHandle(BytesLocation bytesLocation) {
        Objects.requireNonNull(bytesLocation, "Null bytesLocation");
        BytesHandle bytesHandle = new BytesHandle();
        bytesHandle.set(bytesLocation);
        return (DataHandle) bytesHandle;
    }

Could you provide such convenient methods inside FileLocation/ByteLocation classes?

Besides, you see that it turned out to be necessary to use @SupressWarning to make necessary DataHandle<Location>. Maybe you should rework generics in your module to provide ability to return DataHandle<Location> without compiler warnings? Or, at least, please make the methods getFileHandle/getBytesHandle with @SupressWarning a part of your own classes, to avoid your users to write unsafe code.

It is a duplicate of the issue scifio/scifio#508 - it seems here is more suitable place. So you may close that issue in SCIFIO project.

@ctrueden
Copy link
Member

ctrueden commented Aug 6, 2023

you see that it turned out to be necessary to use @SupressWarning to make necessary DataHandle<Location>.

I fixed a generics typing issue with ReadBufferDataHandle (b3b4326) as part of #497 bug-fix, and the 2.95.1 release contains this update. So the SuppressWarnings should no longer be necessary, because you can simply do:

    public static FileHandle getFileHandle(FileLocation fileLocation) {
        Objects.requireNonNull(fileLocation, "Null fileLocation");
        FileHandle fileHandle = new FileHandle();
        fileHandle.set(fileLocation);
        return fileHandle;
    }

    public static BytesHandle getBytesHandle(BytesLocation bytesLocation) {
        Objects.requireNonNull(bytesLocation, "Null bytesLocation");
        BytesHandle bytesHandle = new BytesHandle();
        bytesHandle.set(bytesLocation);
        return bytesHandle;
    }

And then pass the returned DataHandle of whatever subclass directly to the ReadBufferDataHandle constructor type-safely.

I think your approach to avoid creating a Context here is correct (it's what I did also while developing the fix for #467), but these utility methods are more verbose than necessary; it's only constructing the object and then calling set, so I just added a new constructor signature to FileHandle and ByteHandle accepting the location to wrap directly: 0f0cc60, and released scijava-common 2.96.0. I also added the same for URLHandle, part of scijava-io-http (scijava/scijava-io-http@a5b8a17), and released scijava-io-http 0.3.0, in case this is useful to you or others.

@Daniel-Alievsky
Copy link
Author

Unfortunately, it is not enough! In your SCIFIO TiffParser code, "in" stream is declared as
DataHandle<Location> in
And the same type is returned by getStream() method:

	/** Gets the stream from which TIFF data is being parsed. */
	public DataHandle<Location> getStream() {
		return in;
	}

I cannot change the returning type of this method, for example, to DataHandle<? extends Location>, because it is used in other classes, for example, TIFFFormat. Probably other projects also use this method.

It is obviously a fundamental problem of your generics architecture. I've described it in another issue:
#469
My unsafe method is just a workaround, but it is a bad workaround - it "hides" the problem, not resolves it. But, in any case, it is not worse that the existing behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants