Broken referential transparency? about geos HOT 5 CLOSED

Hi there,
Thanks for pointing this out.
This haskell library is a wrapper around the geos library, and as such has to work with the memory management characteristics of the underlying library.
The reason the insert function looks as it does is because the underlying geos data structure is mutable and -- unlike other data structures in geos -- the semantics of the STRTree kind of require it to be mutable.
I have an idea for how to fix your issue -- give me a few days and I'll push something.

from geos.

On reflection,
I think the best move is to remove STRTreeBuilder from the interface (along with its associated functions).
Then, the only function for creating STRTree will be fromList and fromFoldable, which will completely avoid the mutability problem. I'm open to creating other convenient functions for constructing STRTrees. I was considering an unfoldr function, which would have the type signature:
unfoldr :: (b -> Maybe ((Geometry, a), b)) -> b -> STRTree a
Thoughts, @andreabedini ??

from geos.

Hi @ewestern,

I think the simplest approach is to own up to the mutability. How about the following types:

empty :: IO STRTreeBuilder
build :: STRTreeBuilder a -> IO STRTree a 
insert :: Storable a => Geometry b -> a -> STRTreeBuilder a -> IO () 

They might be a way to use ST rather than IO but I haven't done it with FFI yet. I'll investigate.

Edit: this is what I was thinking https://stackoverflow.com/questions/20938819/how-do-i-perform-ffi-calls-inside-st-monad

Edit 2: thinking more about it, perhaps it's a good idea to leave anything in IO and let the user decide whether to use unsafeIOToST. There might be some considerations to make around thread-safety. Admittedly I don't know geos very well.

from geos.

I went ahead with the decision to abandon STRTreeBuilder.
https://hackage.haskell.org/package/geos-0.4.1

I think this approach is best in line with what users will expect when using this library.
Regarding your edit 2, note that I've exposed the "Raw" module, which exposes functions that are much closer to the underlying GEOS api -- these functions run in a Geos monad and can be used if you're looking for a more "honest" abstraction.
That said, the main reason the low-level FFI functions run in IO is because of the effectful logging that geos does. Assuming that this logging and the memory management is handled correctly (which it may not be!), I don't feel too bad about calling unsafePerformIO to exit the Geos monad -- which is what the functions outside of the Raw module end up doing.

Regarding your suggestion to use ST. I'm definitely open to something like that for the Raw module. To be honest, I haven't used ST directly before, so I don't have a good sense for the advantages there.

With this recent patch, I'm going to close this issue. Feel free to open another if you have ideas for functions in the Raw module -- which appears to be closer to what you're looking for.

from geos.

Thanks @ewestern, it sounds like a good step forward. Exposing the Raw module will allow us to experiment with different approaches. Cheers!

from geos.