Expose fromStrict/toStrict directly from Data.ByteString #281
Conversation
| fromStrict, -- :: ByteString -> Lazy.ByteString | ||
| toStrict, -- :: Lazy.ByteString -> ByteString |
There was a problem hiding this comment.
On second thought, in the context of the Data.ByteString module, these seem like better names to me:
| fromStrict, -- :: ByteString -> Lazy.ByteString | |
| toStrict, -- :: Lazy.ByteString -> ByteString | |
| toLazy, -- :: ByteString -> Lazy.ByteString | |
| fromLazy, -- :: Lazy.ByteString -> ByteString |
Data.ByteString.Lazy should keep the old names though.
Thoughts?!
There was a problem hiding this comment.
I like the new names better.
There was a problem hiding this comment.
It feels a bit weird to have the very same function under two different names. Is there any prior art?
Imagine having both modules in scope:
import qualified Data.ByteString as BS
import qualified Data.ByteString.Lazy as BLIf fromStrict is the same entity, just re-exported, GHC suggests only one option for a hole _ :: BS.ByteString -> BL.ByteString - I do not even have to search for this function in haddocks! But if we define toLazy = fromStrict in Data.ByteString, GHC would be obliged to suggest both options. Now this is really confusing for a user: he does not know that it is just synonyms, so needs to check haddocks for both modules and painstakingly compare semantics.
There was a problem hiding this comment.
However, that means that folks who just want to trim the import lists don't win. They have to change their code, and if they want a simple migration path to the new bytestring releases that make the imports optional, they've more work to do.
One might even provide both sets of names. fromStrict == toLazy and fromLazy == toStrict, and re-export both forms from the Internal module. So you can have either name from either source.
There was a problem hiding this comment.
My reason for proposing these names was that I think that with Data.ByteString.toStrict and Data.ByteString.fromStrict it's not obvious which type we're respectively converting from or to. The other type could plausibly be [Word8] or String.
However, that means that folks who just want to trim the import lists don't win. They have to change their code, and if they want a simple migration path to the new
bytestringreleases that make the imports optional, they've more work to do.
@vdukhovni Can you expand on the scenario that you're describing here?
I don't understand how anyone could profit from the new {from,to}Strict re-exports while retaining compatibility with older bytestring versions.
|
Let's move the discussion about I don't like having multiple names for the same thing, because it tends to lead to a chaos. Imagine importing both flavours of import qualified Data.ByteString as BS
import qualified Data.ByteString.Lazy as BLNow for each conversion you have a choice between At the moment I tend to write import qualified Data.ByteString as B
import qualified Data.ByteString.Lazy as B (fromStrict, toStrict)If |
sjakobi
left a comment
There was a problem hiding this comment.
I think the re-exports make sense, even though {from,to}Lazy might be clearer names in the context of Data.ByteString.
If there's demand for adding the latter names, we can still do so. I don't currently have the bandwidth to drive that change though.
|
If eveyone is ok with re-exports, I am keen to merge this before release. |
Works for me. I don't think the "more natural" names actually pan out in practice, so keeping just one set visible from multiple places is roughly optimal. |
Closes #279.
Basically I just moved
fromStrict/toStrictfromData.ByteString.LazytoData.ByteString.Lazy.Internal. The difficulty is that their implementation depends onData.ByteString.{length,null,take,drop}. I could potentially move these four functions toData.ByteString.Internal, but I decided just to operate on fields ofBSdirectly. IMHO it is a better option, because it moves less stuff between modules and also shaves off a couple of picoseconds, avoiding conditional branches intake/drop.