Re: [Discuss-gnuradio] Using the private implementation ("pimpl") pattern for GNU Radio API classes

[Tom Rondeau]

On Tue, Mar 13, 2012 at 11:12 AM, Johnathan Corgan <address@hidden> wrote:

On Tue, Mar 13, 2012 at 01:10, Martin Braun <address@hidden> wrote:

>> However, some implementation details (sorry) we think need to change.
>> Currently, the blocks following this pattern have a public header file
>> based on the block class name, like gr_foo_ff.h, which contains the
>> pure virtual class and nothing else.  Secondly, there exists the
>> implemention class, gr_foo_ff.cc, which contains both the
>> gr_foo_ff_impl class declaration and gr_foo_ff_impl member
>> implementations.
>>
>> Having a class header file inside a .cc file, and then having the name
>> of the .cc file be different from the classes that are inside it,
>> makes it less readable.  Tom and I are proposing, if we do go to a
>> project-wide pimpl pattern, to have:
>>
>> include/gr_foo_ff.h
>> lib/gr_foo_impl_ff.h
>> lib/gr_foo_impl_ff.cc
>
> This might seem minor, but I like the first naming scheme you proposed
> much better than the second (gr_foo_ff_impl > gr_foo_impl_ff).
> I think of 'foo_ff' as the block name, which would be disrupted, and an
> intuitive glob for impl-header-files is (in my head) "*impl.h".

After typing and proofing that long email, it's funny that I could
leave in such a mistake.

It was intended that the filenames listed in the three lines be the
same as the class names two paragraphs above, that was the whole
point.  So yes, exactly as you said.

Thanks for the feedback.

Johnathan

Good catch, Martin, that pattern definitely makes the most sense.

Once we're settled on a general pattern here, we can start planning the changes. We have a few of the new top-level block structures done that are nicely separate and self-contained. We could plan on making the changes there, first. This include gr-digital, gr-vocoder, gr-audio, gr-shd, gr-uhd, and gr-fcd (which should be added soon). This should only take a small amount of effort to do. We'll be leaving everything in gnuradio-core alone for now, though. There's other discussions to be had for what we plan on with that.

So we'll ask any developers to try to keep up as we change things. Nothing's happening right now, and I'd like to try to make this switch happen almost at once. When we move over some of the current directories to this new style, we'll try to push it as one change to master/next. At that point, anyone working on these components would be asked to start using that model. We'll put it on the wiki as well. Of course, if you're actively developing, don't worry much about it; I'll volunteer my time to do the conversion of a new and working block for the time being. I don't want this to be a huge burden.

Thanks for the feedback.

Tom