← Back to team overview

dolfin team mailing list archive

Re: [Branch ~dolfin-core/dolfin/trunk] Rev 6896: Start cleaning up assemblers.

 

On Mon, Sep 10, 2012 at 2:19 PM, Anders Logg <logg@xxxxxxxxx> wrote:
> On Mon, Sep 10, 2012 at 02:08:52PM +0100, Garth N. Wells wrote:
>
>> > No. Parameter names can be easily listed via
>> >
>> >   info(assembler.parameters, True)
>> >
>>
>> That is at runtime. Not when writing code.
>
> Just look at the function default_parameters where they are listed?
>
>> > Then one gets a complete list of which parameters can be set,
>> > including possible values for the parameters. We also get automatic
>> > range checking.
>> >
>> >> Parameters are suitable for options that are passed on the command
>> >> line, are optional and/or might be shared amongst objects.
>> >>
>> >> Using strings over members functions/data unnecessarily is a
>> >> nightmare. It took a lot over work to untangle the crazy string-based
>> >> storage that was used for parallel mesh data so that we could move the
>> >> parallel meshes forward.
>> >
>> > I don't see how that is relevant.
>>
>> Unnecessary use of string-base naming over methods over explicit,
>> named members and data is a poor design and makes working with code
>> confusing.
>
> I completely disagree. The parameter system is good design much better
> than cluttering classes with parameter-like variables.
>
> Another advantage of the parameter system is easier
> initialization. It's a nightmare to initialize a long list of
> parameter variables in each overloaded constructor. (Does not apply
> here since there's only one constructor in this specific case, but I'm
> talking general terms, see below).
>
>> > I very strongly think we should use
>> > the parameter system and not start to clutter the classes with flag
>> > member variables.
>> >
>>
>> Clutter? It's four lines of code, and a lot more than that was
>> removed. Using parameters adds more garbage.
>
> I agree the class in question (AssemblerBase.h/.cpp) looks very pretty
> and neat, not much clutter there, and superbly documented.

Documentation is another point in favour of member, when appropriate.

> I'll give
> you that, but it's a step away from our usual practice which is
> avoiding member variables when we can, and sticking the parameters
> into the parameter system.
>
>> I strongly oppose using the parameter system in this case because it's
>> pointless. If someone else wants to clean up the classes and implement
>> all the functionality that I need (and they do it this week), then I
>> could live with the use of parameters ;).
>
> It's an easy fix to use the parameter system, as you say it's only 4
> lines of code. ;-)
>

It's not just four lines because the parameter system is shaky in
terms of casting. One cannot do:

  if (parameters["foo"])
     . . . .

Garth

> --
> Anders


Follow ups

References