manjian's picture

A Bad Method Signature&Bad Method Naming Idiom

Project:The Open Toolkit library
Version:0.9.8
Component:Code
Category:feature request
Priority:normal
Assigned:manjian
Status:closed
Description

Well.I will talk about a bad method singnature.Its name is GL.GetAttachedShaders.Why I say it's bad.First take a look at the definition of all of its overloaded method.

public static void GetAttachedShaders(int program, int maxCount, int[] count, int[] obj);

public static void GetAttachedShaders(int program, int maxCount, out int count, out int obj);

public static void GetAttachedShaders(uint program, int maxCount, int* count, uint* obj);

public static void GetAttachedShaders(uint program, int maxCount, int[] count, uint[] obj);

public static void GetAttachedShaders(uint program, int maxCount, out int count, out uint obj);

And then let's examine the doc about this function

void GetAttachedShaders( uint program, sizei maxCount,
sizei *count, uint *shaders );
returns the names of shader objects attached to program in shaders. The actual
number of shader names written into shaders is returned in count. If no shaders are attached, count is set to zero. If count is NULL then it is ignored. The maximum
number of shader names that may be written into shaders is specified by maxCount.
The number of objects attached to program is given by can be queried by calling
GetProgramiv with ATTACHED SHADERS.

So, the parameter count should be a single int.I can understand what is int * count, out int count in the parameter list for .But what the hell append on the method whose count type is int[]?Is it supposed to return more than one value?So I have to write the code like the following to make it work?
int[] count=new int[1];
GetAttachedShaders(program, maxCount, count, obj);

That's bad.

Secondly.I wish the coming version deprecate all the unsafe version function.The safe version method is enough to use.The old method naming idiom using pointers would let us write unsafe code.The protection provided by .NET framework would break.And I think its easy to change the unsafe version method to safe version method.


Comments

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
objarni's picture

#1

Since I've asked similar questions to the OpenTK developers before, I might just try to explain why this is a hard problem:

Simplified reason (I don't know the details myself):
1. The OpenGL c API is HUGE
2. OpenTK is basically built using parsers/code generators for the opengl.h-headers, auto-converting C-code to C# code
3. "automagically" creating "intuitive" versions (.NET-ish) of the c-api-functions is a very hard problem, which is left unsolved
4. Instead, "manual work" is done after the C# code for OpenTK has been generated from the c-headers. For example, to make some of the more "esoteric API" more .NET-sane.

I think you found such a "esoteric API".

manjian's picture

#2

I don't think it's a good excute for this.If Microsoft auto-generated their WIN32 API to .NET Framework API without doing carefully redesign work,.NET will surely be a fiasco.So,Let's face this problem,and solve it.

the Fiddler's picture

#3

Status:open» confirmed

Objarni explained the issue pretty well: the resulting API is only as good as the generator and the specs allow.

In this particular case, it might be possible to improve the generator. The function specs define GetAttachedShaders as:

GetAttachedShaders(program, maxCount, count, obj)
	return		void
	param		program		UInt32 in value
	param		maxCount	SizeI in value
	param		count		SizeI out array [1]
	param		obj		UInt32 out array [count]
	category	VERSION_2_0
	dlflags		notlistable
	version		2.0
	extension
	glxsingle	?
	glxflags	ignore
	glsflags	get
	glsopcode	?
	offset		?

Parameter count is an out array containing one element, which is a relatively strange definition (why is it not an out pointer if it is supposed to contain just a single element?)

Right now the generator ignores the array size, but it is possible to parse and use - maybe not in the general case, but only when it is 1.

objarni's picture

#4

Is the code generator what lies beneath the "Bind" folder in the source distribution of OpenTK?

the Fiddler's picture

#5

Status:confirmed» fixed

Yes (warning: the code is insane).

Fixed in rev. 1698 at approximately 33000 feet above ground (high-level debugging).

the Fiddler's picture

#6

Version:0.9.7» 0.9.8
Status:fixed» closed

Closing issues resolved in 0.9.8.