Re: feedback asked on dab-decoder software in Ada

[jan van katwijk <j.vankatwijk@gmail.com>]

Op zaterdag 24 september 2016 19:45:26 UTC+2 schreef jan van katwijk:
> Op donderdag 15 september 2016 11:00:10 UTC+2 schreef Jacob Sparre Andersen:
> > Jan van Katwijk wrote:
> > 
> > > This summer I wanted to learn Ada (again, after a period of well over
> > > 20 years) and I made a reimplementation of the DAB software in Ada.
> > 
> > Sounds like an ambitious project.
> > 
> > > I would like to get some feedback on the use of the Ada language.
> > 
> > Some comments and questions:
> > 
> > + It would make it easier to get contributions from other Ada
> >   developers, if you switched source style to something closer
> >   to what is suggested by the Ada Quality and Style Guide [1].
> > 
> > + Why do you put the package specifications in a separate directory?
> > 
> > + You might benefit from running your compiler with more warnings
> >   enabled.
> > 
> > + A package body doesn't have to "with" itself.
> > 
> > + There are no guarantees that "Integer" in Ada is the same as "int" in
> >   C.  If you need a C "int", you should use "Interfaces.C.int".
> > 
> > + Are you sure you need as many access types as you declare?  (It looks
> >   - understandably - a bit like you are writing C in Ada.)
> > 
> > + It looks like you aren't getting as much out of the type system as you
> >   could.  (A length should probably not be able to contain negative
> >   values. - Just to take a single example.)
> > 
> > > Any feedback and suggestions for improvement (it definitely runs
> > > slower than the C++ version) is welcome
> > 
> > I would suggest that you postpone the performance improvements a bit,
> > and focus on getting more out of Ada.
> > 
> > Greetings,
> > 
> > Jacob
> > 
> > [1] http://www.adaic.org/resources/add_content/docs/95style/html/cover.html
> > -- 
> > "For there are only two reasons why war is made against a
> >  republic: The one, to become lord over her: the other, the
> >  fear of being occupied by her."       -- Nicolo Machiavelli
> 
> Thanks for the comments. In the meantime I tried to apply some guideline rules, and the gnat compiler does not complain anymore on the layout.
> Wrt identifiers: I am not impressed very much by the guidelines, to me Is_Running is not better readable than isRunning, Has_Property not than hasProperty.
> 
> First of all: the ada implementation of DAB is derived from the existing C++ implementation, all algorithms were extensively tried and tested in C++, so the complexity was more on using C libraries (I use several C libraries), some of them with callbacks. That all went pretty smooth, however, handling the GUI turned out to be more difficult than hndling the much more complex GUI in C++ (GtkAda vs Qt) 
> 
> The code and coding style is obviously influenced by having programmed in C++ for the last 10 years. Wrt access types, I really have to look at it, most likely it follows from the C++ background.
> 
> Anyway, thanks for the advices, I'll continue to make it more "Ada-style"
> 
> jan

I looked into the abundant use of access types. In the front end (i.e. the ofdm processing part) some access varaibles could be eliminated and replaced by "normal" variables. In the backend, however, quite some dynamic choices determine which part of the program should be active (e.g. MP2 decoding or MP4 decoding).

jan