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!feeder2.cambriumusenet.nl!feed.tweaknews.nl!193.201.147.71.MISMATCH!xlned.com!feeder3.xlned.com!feeder.erje.net!news2.arglkargh.de!news.tornevall.net!not-for-mail From: "Jeffrey R. Carter" Newsgroups: comp.lang.ada Subject: Re: This MIDI stuff, would someone be interested in reviewing my code? Date: Mon, 08 Mar 2010 10:24:26 -0700 Organization: TornevallNET - http://news.tornevall.net Message-ID: References: NNTP-Posting-Host: 044d68403db18513f7a807b54c7950fd Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Trace: edb5054bd09cb2b54f3c5cada12e1e80 X-Complaints-To: abuse@tornevall.net X-Complaints-Language: Spoken language is english or swedish - NOT ITALIAN, FRENCH, GERMAN OR ANY OTHER LANGUAGE! In-Reply-To: X-Validate-Post: http://news.tornevall.net/validate.php?trace=edb5054bd09cb2b54f3c5cada12e1e80 X-SpeedUI: 1738 X-Complaints-Italiano: Parlo la lingua non � italiano User-Agent: Thunderbird 2.0.0.23 (X11/20090817) X-Posting-User: 0243687135df8c4b260dd4a9a93c79bd Xref: g2news1.google.com comp.lang.ada:9471 Date: 2010-03-08T10:24:26-07:00 List-Id: 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