:: Re: [Libbitcoin] [PATCH] Implement …
Top Pagina
Delete this message
Reply to this message
Auteur: Amir Taaki
Datum:  
Aan: Libbitcoin
Onderwerp: Re: [Libbitcoin] [PATCH] Implement a bitcoin URI parser
Also I wouldn't worry too much about performance :) Far better is clarity,
simplicity and directness of code. For instance the only place in
libbitcoin I'm concerned about performance is the blockchain and maybe the
memory overhead for the network stuff. Especially for parsing user data
(like a URI) which you won't be doing millions of times a second, it
doesn't matter too much. So in this case we might choose to write code
that isn't as performant as it could be because it looks pretty.

> On Wed, Mar 19, 2014 at 8:24 PM, Amir Taaki <genjix@???> wrote:
>> btw why does uri_parse() need to be publically exposed when you already
>> have uri_decode()? can we hide that implementation detail?
>
> If anyone wants to extend the bitcoin URI scheme in the future, this
> provides a way to get at the raw parameters. Otherwise, people would
> be forced to patch uri_decode for every little thing, including things
> that may be experimental or not appropriate for libwallet.
>
> On Wed, Mar 19, 2014 at 8:21 PM, Amir Taaki <genjix@???> wrote:
>> Pull request: https://github.com/spesmilo/libwallet/pull/9
>> Accept if you like my changes.
>
> Ooooh, boost::optional is exactly what I was looking for. I couldn't
> find anything like it in the standard library, so I gave up and went
> with the bool thing.
>
> I come from an embedded firmware environment where memory is precious,
> so I always go for things like the visitor pattern to avoid allocating
> data structures. Your way is more straightforward, but now we have
> this uri_parse_result that needs to be thrown away.
>
> Some of the tests you commented out were actual edge cases, so I will
> go back through and make sure we have comprehensive tests again.
>
> Apart from the missing unit tests, I like your changes and will be
> incorporating them shortly.
>
> In the mean time, I figured out a better to handle malformed URI's.
> The original solution felt wrong, since it gives the space character
> special treatment for no good reason. My new solution adds a "strict"
> flag, which determines whether the parser will follow the escaping
> rules exactly, or whether it will completely ignore them in the case
> of an unencoded parameter. Since I've pushed these changes up to
> master, your pull request won't apply cleanly, so I might as well fix
> the unit tests as long as I'm solving merge conflicts.
>
> -William
>