Prev: TIOBE index
Next: Bitbucket.org now supports Ada and Modula-2 syntax highlighting
From: Jeffrey R. Carter on 8 Mar 2010 12:24
John McCabe wrote:
>
> 1) Perhaps I should be using limited withs in some places to get
> access to the primitive operators/functions of the stuff in
> Interfaces.C/.strings and Win32 etc.

That's not what "limited with" is for.

I notice that you "use" a lot of pkgs, but then refer to things using full
names. You should get rid of those "use" clauses, and add "use type" clauses in
your declarative part as needed.

> 3) Would it be more appropriate to use something like
> Win32.UINT'Image() instead of getting an instantiation of the
> Modular_IO package?

That's a matter of taste and needed functionality.

My first comments would be to use common Ada naming conventions: Dev_ID,
Num_Input_Devices, and so on.

I also prefer to see "loop" and "then" on the same line as "for" and "if". This
looks like a C-family person thinking they're equivalent to '{'.

I would make Numinputdevices and Numoutputdevices constants. For this small
example, I would do the same with Midiincaps and Midioutcaps; there's really no
need to free them, since they're around for the entire program, and will be
freed when the program exits.

> with Interfaces.C; use Interfaces.C;
> with Interfaces.C.Strings; use Interfaces.C.Strings;

"with Interfaces.C.Strings;" implies "with Interfaces; with Interfaces.C;", so
there's no reason to have both.

> with Win32; use Win32;
> with Win32.Mmsystem; use Win32.Mmsystem;

Ditto.

> for devId in Win32.UINT range 0..(NumInputDevices - 1)
> loop

These parentheses are unnecessary.

> res := Win32.Mmsystem.midiInGetDevCaps(devId,
> midiInCaps,
>
> (Win32.Mmsystem.MIDIINCAPS'size * Win32.BYTE'size));

So are the internal ones here.

You've already been warned about sprinkling OS-dependent stuff throughout your code.

--
Jeff Carter
"C's solution to this [variable-sized array parameters] has real
problems, and people who are complaining about safety definitely
have a point."
Dennis Ritchie
25
From: Christophe Chaumet on 13 Mar 2010 03:12
John McCabe a �crit :
> Hi
>
> It's still early days but, if I'm going to be using Ada to try to
> build this app I want, it would be nice to write it in a style that
> looks appropriate. I'm aware of the Q&S guide but I was hoping that
> someone could take a quick look at the code I've written (it's only 80
> lines or so, and it's down below) and see if there's anything
> obviously stupid I'm doing.
>
> My specific thoughts on this are:
>
> 1) Perhaps I should be using limited withs in some places to get
> access to the primitive operators/functions of the stuff in
> Interfaces.C/.strings and Win32 etc.
>
> 2) The for loops: for devId in Win32.UINT range 0..(NumOutputDevices -
> 1) etc. These are protected by a "if NumOutputDevices < 0" condition
> but before I realised my mistake I found that when NumOutputDevices is
> 0, the loop executes as many times as it can before it crashed. This
> was obviously because NumOutputDevices was 0, so the range
> "0..(NumOutputDevices - 1)" was 0..4929blahblah due to Win32.UINT
> being a modular type. I looked at the option to use something like:
> for index in Win32.UINT range 1..NumOuputDevices loop
> declare
> devId : Win32.UINT := index - 1;
> begin
> ...
> end;
> end loop;
> but stuck with the original with the conditional round it.
>
> 3) Would it be more appropriate to use something like
> Win32.UINT'Image() instead of getting an instantiation of the
> Modular_IO package?
>
> Anyway - thanks to anyone who can be bothered to look at this. It will
> be much appreciated, and thanks for everyone's help so far.
>
> John
>
>
> ===================
> with Ada.Text_IO;
> with Ada.Unchecked_Deallocation;
>
> with Interfaces.C; use Interfaces.C;
> with Interfaces.C.Strings; use Interfaces.C.Strings;
>
> with Win32; use Win32;
> with Win32.Mmsystem; use Win32.Mmsystem;
>
> procedure MidiDevs is
> NumInputDevices : Win32.UINT;
> NumOutputDevices : Win32.UINT;
>
> res : Win32.Mmsystem.MMRESULT;
> midiInCaps : Win32.Mmsystem.LPMIDIINCAPS;
> midiOutCaps : Win32.Mmsystem.LPMIDIOUTCAPS;
>
> package UINTText_IO is new Ada.Text_IO.Modular_IO(Win32.UINT);
> package MMText_IO is new
> Ada.Text_IO.Modular_IO(Win32.Mmsystem.MMRESULT);
>
> procedure Free is new
> Ada.Unchecked_Deallocation(Win32.Mmsystem.MIDIINCAPS,
> Win32.Mmsystem.LPMIDIINCAPS);
> procedure Free is new
> Ada.Unchecked_Deallocation(Win32.Mmsystem.MIDIOUTCAPS,
> Win32.Mmsystem.LPMIDIOUTCAPS);
>
> begin
> NumInputDevices := Win32.Mmsystem.midiInGetNumDevs;
> NumOutputDevices := Win32.Mmsystem.midiOutGetNumDevs;
> midiInCaps := new Win32.Mmsystem.MIDIINCAPS;
> midiOutCaps := new Win32.Mmsystem.MIDIOUTCAPS;
>
> Ada.Text_IO.Put("There are ");
> UINTText_IO.Put(NumInputDevices, 0);
> Ada.Text_IO.Put(" input devices available, and ");
> UINTText_IO.Put(NumOutputDevices, 0);
> Ada.Text_IO.Put_Line(" output devices available.");
>
> if NumInputDevices > 0
> then
> Ada.Text_IO.New_Line;
> Ada.Text_IO.Put("The ");
> UINTText_IO.Put(NumInputDevices, 0);
> Ada.Text_IO.Put_Line(" input devices are:");
> Ada.Text_IO.New_Line;
>
> for devId in Win32.UINT range 0..(NumInputDevices - 1)
> loop
> res := Win32.Mmsystem.midiInGetDevCaps(devId,
> midiInCaps,
>
> (Win32.Mmsystem.MIDIINCAPS'size * Win32.BYTE'size));
> UINTText_IO.Put(devId, 0);
> Ada.Text_IO.Put(") ");
> if res = Win32.Mmsystem.MMSYSERR_NOERROR
> then
> Ada.Text_IO.Put("szPname = ");
> Ada.Text_IO.Put_Line(To_Ada(To_C(midiInCaps.szPname)));
> else
> Ada.Text_IO.Put("Query Failed. Returned ");
> MMText_IO.Put(res, 0);
> end if;
> Ada.Text_IO.New_Line;
> end loop;
> end if;
>
> if NumOutputDevices > 0
> then
> Ada.Text_IO.New_Line;
> Ada.Text_IO.Put("The ");
> UINTText_IO.Put(NumOutputDevices, 0);
> Ada.Text_IO.Put_Line(" output devices are:");
> Ada.Text_IO.New_Line;
>
> for devId in Win32.UINT range 0..(NumOutputDevices - 1)
> loop
> res := Win32.Mmsystem.midiOutGetDevCaps(devId,
> midiOutCaps,
>
> (Win32.Mmsystem.MIDIOUTCAPS'size * Win32.BYTE'size));
> UINTText_IO.Put(devId, 0);
> Ada.Text_IO.Put(") ");
> if res = Win32.Mmsystem.MMSYSERR_NOERROR
> then
> Ada.Text_IO.Put("szPname = ");
> Ada.Text_IO.Put_Line(To_Ada(To_C(midiOutCaps.szPname)));
> else
> Ada.Text_IO.Put("Query Failed. Returned ");
> MMText_IO.Put(res, 0);
> end if;
> Ada.Text_IO.New_Line;
> end loop;
> end if;
>
> Free(midiInCaps);
> Free(midiOutCaps);
>
> end MidiDevs;
>
>
Here is a working code: http://sourceforge.net/projects/canta/ written
in Ada.
 | 
Pages: 1
Prev: TIOBE index
Next: Bitbucket.org now supports Ada and Modula-2 syntax highlighting