[m-rev.] for review: eliminate some code duplication in the opengl binding

Fergus Henderson fjh at cs.mu.OZ.AU
Wed Jan 14 01:03:11 AEDT 2004


On 13-Jan-2004, Julien Fischer <juliensf at students.cs.mu.OZ.AU> wrote:
> 
> Estimated hours taken: 0.5
> Branches: main
> 
> This was suggested by Fergus in his review of the last change.
> 
> Rewrite the bindings for mogl.enable/3, mogl.disable/3 and mogl.is_enabled/4
> in order to eliminate some code duplication.  The new versions are smaller
> and should (hopefully) also be more robust when/if we ever getting around
> to adding features from versions of OpenGL > 1.1.
> 
> extras/graphics/mercury_opengl/mogl.m:
> 	Rewrite the bindings for mogl.enable/3, mogl.disable/3
> 	and mogl.is_enabled/3.

The summary in the log message is a little longer than it needs to be.
A simpler summary, e.g. just "Refactor some code in the OpenGL interface
to improve maintainability.", would be preferable.  So I suggest the
following log message:

	Estimated hours taken: 0.5
	Branches: main

	Refactor some code in the OpenGL interface to improve maintainability.

	extras/graphics/mercury_opengl/mogl.m:
		Rewrite the bindings for mogl.enable/3, mogl.disable/3
		and mogl.is_enabled/4 in order to eliminate some code
		duplication.  The new versions are smaller and should
		(hopefully) also be more robust when/if we ever getting
		around to adding features from versions of OpenGL > 1.1.

> Index: mogl.m
> @@ -2857,99 +2858,47 @@
>  ").
> 
>  enable(Flag, !IO) :-
> +	control_flag_to_int_and_offset(Flag, Int, Offset),
> +	enable_2(Int, Offset, !IO).
> 
> +:- pred enable_2(int::in, int::in, io::di, io::uo) is det.
>  :- pragma foreign_proc("C",
> +	enable_2(Flag::in, Offset::in, IO0::di, IO::uo),

This is potentially a bit confusing, since the name "Flag" is used
in enable/3 to denote a value of type control_flag and in enable_2/4
to denote a value of type int.

I suggest s/Flag/FlagVal/ in enable_2/4.
and perhaps also s/Int/FlagVal/ in enable/3.

Likewise for the other functions.

Otherwise, that change looks good.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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