kicad-developers team mailing list archive

[Simon Richter <Simon.Richter@xxxxxxxxxx>]

On 27.06.2016 23:47, Michael Steinberg wrote:

> === modified file 'bitmap2component/bitmap2cmp_gui.cpp'
> --- bitmap2component/bitmap2cmp_gui.cpp	2016-06-08 06:32:01 +0000
> +++ bitmap2component/bitmap2cmp_gui.cpp	2016-06-27 19:02:56 +0000
[...]
> +    // TODO: Should the config really not be written if BM2CMP is
iconized?

Good question.

Meta-question: Should we use /// @todo for these to get doxygen to
compile a list?

> -    delete m_config;
> +    m_config->Flush();
> +    m_config.reset();

In the long run, we'd need to have error handling code here. As it is
now, the patch is certainly an improvement over the old state, where a
destructor was tasked with flushing the config to disk, and had no
recourse on error.

[...]

> -    cfg = new wxFileConfig( wxT( "" ), wxT( "" ),
configname.GetFullPath() );
> -    return cfg;
> +    return std::unique_ptr<wxConfigBase>(
> +        new wxFileConfig(wxT(""), wxT(""), configname.GetFullPath())
> +    );

C++14 has std::make_unique, but we'll cross that bridge when we reach it.

> -        wxConfigBase* config = GetNewConfig( fn.GetFullPath() );
> +        auto config = GetNewConfig( fn.GetFullPath() );
>          config->Write( HOTKEYS_CONFIG_KEY, msg );
> -        delete config;

This has subtle potential for errors -- while I'm normally in favour of
using "auto", spelling it out would help here in case the function
signature is changed back.

   Simon

Attachment: signature.asc
Description: OpenPGP digital signature