[darcs-users] index format

Jason Dagit dagit at codersbase.com
Mon Jun 8 15:08:25 UTC 2009


On Mon, Jun 8, 2009 at 12:11 AM, Petr Rockai <me at mornfall.net> wrote:

> Jason Dagit <dagit at codersbase.com> writes:
> > I take it that you've defined (?) with the type:
> > (?) :: Bool -> (a, a) -> a
> Yes.
>
> > Is the above real code or pseudo code?
> Real code.
>
> > The use of 'undefined' and explicitly checking for '/' seem like
> potential
> > bugs waiting to happen.
> The only place where dirlen is Nothing is when peekItem is called from
> createItem. In this case, iName is never used, so the code is safe. Both
> createItem and peekItem are supposed to be private in the Index module.
> (I'll
> add a remark that they are unsafe and I'll add explicit export list to the
> module).


My recommendation is to make that field hold a Maybe instead of the raw
value.  I can see myself, or someone else, not understanding this invariant
and 'accidentally' using that slot when it is undefined.  If you instead use
Maybe then it is much easier to see how to use this safely.  Your
description above makes me quite concerned about the robustness of the code.


>
>
> > Using fromJust seems equally risky, and I see that you guard it and then
> use
> > undefined.  Why do you prefer undefined over a fromJust failure?  In
> > particular, the fromJust that darcs provides gives you line number
> > information becase it is implemented as a CPP macro.
> Ok. I guess "error" would be even more appropriate here. I don't think that
> propagating the maybe-ness further makes much sense, since the use pattern
> for
> iName is determined statically. It would just be inconvenient to describe
> that
> in the type system, so it's not.


This is darcs, we like to exploit the type system :)


>
>
> > Also, do you provide justification for your constants?  eg., 4, 8, 64.
> Would
> > those constants change on other platforms?
> 64 = length of sha256 as hex string
> 4 = length of Int32
> 8 = length of Int64
>
> These are all platform-independent.


Thanks.  Perhaps you could create top level names for those?

Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20090608/7bd42374/attachment.htm>


More information about the darcs-users mailing list