[m-rev.] for review: add transform_value predicate to map.

Julien Fischer juliensf at cs.mu.OZ.AU
Wed Jan 12 20:35:31 AEDT 2005


On Wed, 12 Jan 2005, Ian MacLarty wrote:

> On Wed, Jan 12, 2005 at 07:54:04PM +1100, Julien Fischer wrote:
> >
> > On Wed, 12 Jan 2005, Ian MacLarty wrote:
> >

> >
> > > Index: NEWS
> > > ===================================================================
> > > RCS file: /home/mercury1/repository/mercury/NEWS,v
> > > retrieving revision 1.360
> > > diff -u -r1.360 NEWS
> > > --- NEWS	11 Jan 2005 06:06:14 -0000	1.360
> > > +++ NEWS	12 Jan 2005 07:32:41 -0000
> > > @@ -234,7 +234,7 @@
> > >    variables.  We've also added a function version.
> > >
> > >  * We've added some new predicates, map__common_subset, map__foldl3,
> > > -  and map__overlay_large_map, to map.m.
> > > +  map__overlay_large_map and map__transform_value, to map.m.
> > >
> > >  * We've added a predicate, map_fold, to set.m.
> > >
> > This doesn't mention the changes to the rbtree module.
> >
>
> Is that news worthy?  Or is every change to the library supposed to go into the
> NEWS file?
>
I think it's worth mentioning.

> > > +:- pred map__transform_value(pred(V, V)::in(pred(in, out) is det), K::in,
> > > +	map(K, V)::in, map(K, V)::out) is semidet.
> >
> > I think you should also add a mode where the closure is semidet.
> >
>
> I don't know if that would be particularly useful.  If transform_value failed
> how would you know if the key wasn't found or the transformation failed?
>
Good point, although it may be useful with the det version in that case.
I guess it doesn't matter too much, if it's ever needed it won't be too
difficult to add.

> > > +
> > > +	% Same as transform_value/4, but aborts instead of failing if the
> > > +	% key is not found.
> > > +	%
> >
> > I prefer "throws an exception" to "aborts"
> >
>
> "aborts" is used elsewhere in the module to describe the det versions of
> predicates, so I think that convention should be followed for consistency.
>
I suspect "aborts" originated when calling error/1 really did abort
execution - I think saying it throws an exception is probably more
accurate now.

> > > +:- pred map__det_transform_value(pred(V, V)::in(pred(in, out) is det), K::in,
> > > +	map(K, V)::in, map(K, V)::out) is det.
> > > +:- func map__det_transform_value(func(V) = V, K, map(K, V)) = map(K, V).
> > > +
> >
> > Our current coding standards say that if there is both a predicate
> > and a function version of a procedure then the :- func declaration
> > should appear first.
> >
>
> Elsewhere in the module preds appear before functions.
>
That's because this module was written way before the above was
added to the coding standard - feel free to shuffle the others
around as well ;-)

> > > +:- pred tree234__transform_value(pred(V, V)::in(pred(in, out) is det), K::in,
> > > +	tree234(K, V)::in, tree234(K, V)::out) is semidet.
> > > +
> > >  	% count the number of elements in a tree
> > >  :- pred tree234__count(tree234(K, V), int).
> > >  :- mode tree234__count(in, out) is det.
> > > @@ -774,6 +781,104 @@
> > >  			;
> > >  				Result2 = (>),
> > >  				tree234__update(T3, K, V, NewT3),
> > > +				Tout = four(K0, V0, K1, V1, K2, V2,
> > > +					T0, T1, T2, NewT3)
> > > +			)
> > > +		)
> > > +	).
> > > +
> >
> > What is this change here?
> >
>
> Because the code for transform_value is almost the same as update
> diff highlighted the wrong bit as new.
>
Fair enough.

Cheers,
Julien.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list