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

[Ian MacLarty]

On Wed, Jan 12, 2005 at 07:54:04PM +1100, Julien Fischer wrote:
> 
> On Wed, 12 Jan 2005, Ian MacLarty wrote:
> 
> > For review by anyone.
> >
> > Estimated hours taken: 2
> > Branches: main
> >
> > Add transform_value predicate to map, tree234 and rbtree.  This applies
> 
> I suggest: Add the operation transform_value to ....
> (since there are both predicate and function versions).
> 

Okay.

> > a higher order argument to a value in the map.  Often a value needs to be
> > updated using its previous value, currently requiring two lookups of the
> > key.  With transform_value only one lookup is required.
> >
> Make the last bit here a separate sentence, e.g.
> 
> 	This currently reuiqres two lookups of the key.  etc
> 

Okay.

> 
> > 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?

> > Index: library/map.m
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/library/map.m,v
> > retrieving revision 1.93
> > diff -u -r1.93 map.m
> > --- library/map.m	10 Jan 2005 05:23:46 -0000	1.93
> > +++ library/map.m	12 Jan 2005 02:25:33 -0000
> > @@ -124,6 +124,20 @@
> >  :- pred map__det_update(map(K, V)::in, K::in, V::in, map(K, V)::out) is det.
> >  :- func map__det_update(map(K, V), K, V) = map(K, V).
> >
> > +	% Update the value at the given key by applying the supplied
> > +	% transformation to it.  Fails if the key is not found.  This is faster
> > +	% that first searching for the value and then updating it.
> > +	%
> e/that/than/ above
> 

Okay

> > +:- 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?

> > +
> > +	% 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.

> > +:- 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.

> > +:- 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.

Ian.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------