astrojs / fitsjs

JavaScript library for reading the FITS astronomical format. Functionality includes reading of images, data cubes, compressed images, binary and ascii tables.
http://astrojs.github.io/fitsjs/
MIT License
85 stars 21 forks source link

Use a wrapperclass for DataView instead of extending it #7

Closed nddeluca closed 11 years ago

nddeluca commented 11 years ago

I may be wrong here, but it might be more effective to create your own DataView class that is used as a wrapper for the native javascript DataView object. Then instead of prototyping new methods, you can define your own functions that wrap the native DataView methods.

This will enable to you pass the bitpix paramter to your wrapper class and keep all of the logic of using the right accessor in the wrapper class - instead of cluttering up the File and Image objects with it.

I think that would result in making the code more readable and then you'll be able implement Issue #5 more easily.

kapadia commented 11 years ago

Agreed. Thanks for the input. Initially I was using a shim for the DataView object (jDataView). The API for the shim is exactly how I extended the native DataView object (e.g. saving a pointer within DataView among other attributes), which saved me from updating the rest of fitsjs.

A more lean approach would be to save the needed variables on the FITS.File object or, as you point out, a wrapper for DataView.

kapadia commented 11 years ago

If you have a pull request, I'm happy to look at it. Before I address this issue, I plan to change other aspects of the library.

nddeluca commented 11 years ago

Okay cool - I'll see what I can come up with and submit a pull request

kapadia commented 11 years ago

Done. Offset is managed in each dataunit class.