From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on polar.synack.me X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=ham autolearn_force=no version=3.4.4 X-Google-Thread: a07f3367d7,5d6e2ad0b6f4137a X-Google-Attributes: gida07f3367d7,public,usenet X-Google-NewGroupId: yes X-Google-Language: ENGLISH,ASCII-7-bit Path: g2news1.google.com!news4.google.com!feeder3.cambriumusenet.nl!feeder1.cambriumusenet.nl!feed.tweaknews.nl!195.238.0.231.MISMATCH!news.skynet.be!aioe.org!not-for-mail From: John McCabe Newsgroups: comp.lang.ada Subject: Re: This MIDI stuff, would someone be interested in reviewing my code? Date: Tue, 09 Mar 2010 10:21:00 +0000 Organization: Aioe.org NNTP Server Message-ID: References: NNTP-Posting-Host: RXEkuaSUwmKe0XIGFYSK7A.user.speranza.aioe.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Complaints-To: abuse@aioe.org X-Notice: Filtered by postfilter v. 0.8.2 X-Newsreader: Forte Agent 2.0/32.652 Xref: g2news1.google.com comp.lang.ada:9482 Date: 2010-03-09T10:21:00+00:00 List-Id: On Mon, 08 Mar 2010 10:24:26 -0700, "Jeffrey R. Carter" wrote: >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. Ah - thanks. I was getting confused there between limited with and use type. You've clarified that for me. >> 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. Ok - I'll see what I can do. >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 was thinking of them being more equivalent to Ada's begin for loops. I like the idea of having a visual clue, be it a begin, loop, then, { or whatever on a line on its own to show clearly that the following code is indented because it's within that construct. >I would make Numinputdevices and Numoutputdevices constants. They're not constants though - they're discovered from the midiInGetNumDevs and midiOutGetnumDevs functions. >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. The point of allocating/freeing them was to get round the fact that, when they were declared in the declarative part of the main function, I seemed to have to use 'Unchecked_Access when passing them to midiInGetDevCaps/midiOutGetDevCaps. >> 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. Ok - thanks. >> with Win32; use Win32; >> with Win32.Mmsystem; use Win32.Mmsystem; >Ditto. Ditto too :-) >> for devId in Win32.UINT range 0..(NumInputDevices - 1) >> loop > >These parentheses are unnecessary. Yes, I agree. I like to make things obvious though :-) >> res := Win32.Mmsystem.midiInGetDevCaps(devId, >> midiInCaps, >> >> (Win32.Mmsystem.MIDIINCAPS'size * Win32.BYTE'size)); > >So are the internal ones here. True. >You've already been warned about sprinkling OS-dependent stuff throughout your code. Yip. Thanks for those comments. Much appreciated.