Hey Carter,

I really appreciate the reply. This is exactly the type of feedback that I find very helpful, as it helps me become a better Haskell programmer. To address your points:

1. Great catch about making a copy of the input. I didn't notice the potential for space leaks here, but what you're saying makes total sense. I just added a call to Data.Text.copy around the inputs of the validation functions, so that should mitigate this risk. I released this as isbn-1.1.0.1 on Hackage: https://hackage.haskell.org/package/isbn-1.1.0.1

2. In terms of the changing the internal representation of ISBN to be something other than Text, this is definitely something I'm open to, since it's not that hard to think of a situation where someone is working with a large number of ISBNs and space use matters. I like your suggestion to use Word64, but this would be the first time I have worked with Word64 in this manner, so I'll have to look into it a bit more. I did also receive some other feedback from someone else about the exact same thing. I incorporated your feedback and the other feedback into an issue in the GitHub repo if you'd like to take share your thoughts: https://github.com/charukiewicz/hs-isbn/issues/2

Thanks again,

Christian



‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Thursday, July 30, 2020 9:04 AM, Carter Schonwald <carter.schonwald@gmail.com> wrote:

cool! Thanks for sharing!
(i mean, who doesn't like to read a good book!)

Question: would it be worth considering  internally having the digits be stored as half bytes for space usage? eg ISBN10 -> use ~ 5 bytes, and ISBN13 be ~ 7 bytes  (this could be within a word64 as a tiny 8 slot array, OR  or any sort of vector/array datatype)? (or instead of a decimal binary rep, pack it into a word64 as the number itself for either?) Granted this would complicate some of the internals engineering a teeny bit

it looks like, in the current code, if there's  no hyphens, it'll keep the input text value as the internal rep, *which can cause space leaks* if it's a slice of a much larger text input you otherwise do not intend to retain.  When it generates its own text string, well...  the text package uses utf16 (aka 2 bytes per character for valid ascii) as the internal rep, so the buffer representation  will occupy 10 * 2 bytes  or 13* 2bytes, so 20-26 bytes within the buffer, ignoring the extra word or two of indexing/offsets!

point being ... it seems like could embed them (with a teeny bit of work) as follows

data ISBN = 
     IsIBSN10 ISBN10 
     | isISBN13 ISBN13

newtype ISBN10 = ISBN10 Word64
newtype ISBN13 = ISBN13 Word64

and then i'd probably be inclined to do use Data.Bits and do the "word 64 as a 16 slot array of  word4's," which would also support the base 11 digit at the end encoding constraint, since word4 == base 16 :) 
then a teeny bit of work to do the right "right" ords and shows etc on this rep etc

i hope the design i'm pointing out makes sense for you (and if i'm not being clear, please holler and i'll try to help)

and again, thanks for sharing this! 
-Carter


On Wed, Jul 29, 2020 at 1:39 PM Christian Charukiewicz via Haskell-Cafe <haskell-cafe@haskell.org> wrote:
Hello Haskell Cafe,

I wanted to share my first ever Haskell package: isbn

https://hackage.haskell.org/package/isbn

The package is motivated by my need to validate ISBNs (the unique identifier associated with every book published since 1970) in a Haskell application I am building. I published isbn as a back in May but yesterday I made some improvements the API and I think it is now ready to share as v1.1.0.0.

I have been using Haskell commercially for a few years, and have made several contributions to various packages, but as mentioned, this is my first time authoring and publishing a package. If anyone has any feedback, I would be happy to hear it.

Thank you,

Christian Charukiewicz
_______________________________________________
Haskell-Cafe mailing list
To (un)subscribe, modify options or view archives go to:
http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
Only members subscribed via the mailman list are allowed to post.