[Aptitude-devel] tabs-manager-eliminate-casts-qt

[Daniel Burrows]

On Tue, Jul 27, 2010 at 12:00:04PM +0200, Piotr Galiszewski <piotr at galiszewski.pl> was heard to say:
> On the other hand tabs_manager::tabs_manager_impl::tab_destroyed()
> method doesn't work in actual code. I discovered this during writing
> Update Cache tab, the first tab, which is for real benefit from this
> method. I changed this in the same patch. New version doesn't use
> qobject_cast. Is it allowed that this patch come later, or should I
> rebase all series and send this chunk earlier? There are also some
> casts in tab_deletion_requested, but they could be easily removed.

  I think it's fine to send that later.  I like to avoid fixup patches
when possible, but once something is in the tree, we're stuck with that
anyway.  I would prefer to separate those changes into their own commit
if it's not too much trouble.

> I also decided that all new classes I am writing are hidden behind
> abstract interface, as in other aptitude's code. I am not rewriting
> eralier patches, because without problems it could be done later
> (interfaces for other classes doesn't changed). I use create_object()
> methods. I am not sure what is preffered. Functions outside class, or
> static create methods in class?

  I waffle around on this in the rest of the codebase, as you may have
noticed.  This is probably a good time to make a decision.

> For tabs I will use ::create()
> methods, so create_or_display will work without problems.

  That's a good point.  Metaprogramming likes having nested names
instead of uniformly chosen (but different) non-nested names.  The
practical argument in the other direction (functions outside class) is
that you don't have to expose the class at all.

  I think that in my existing code I tend to divide interfaces into
two types: those that are intended to have only one concrete
implementation (at least for "production"), and those that are intended
to have multiple concrete implementations.  In the first case, I use a
make() or create() static method, and in the second I use a create()
method outside the class.

  So I would make the call based on what type of interface we're
creating an instance of.  I'll add a README.INTERFACES to the codebase
to describe how interfaces are used in aptitude.

> Another question: What brackets' policy should I use in my code. I am
> still little confused about this ;):
> if()
>   {
>     some_method();
>   }

  This one.

> if()
> {
>   some_method();
> }

  But this is what's used for some other stuff (most notably methods
and classes).

  This is all basically the default settings of emacs for C++ coding.

> Probably it is also good time to write something about my current
> work. As DebConf is fast approaching I  am concentrating now on
> features development (I'd like to have as much functions as it is
> possible to work before presentation about project). After
> conference's starting I will come back to normal coding (including
> refactoring and writing missing comments ;) )

  OK.  I'll keep going over your older patches to merge them in.  As
you may have noticed, for some of the smaller stuff I'm just committing
fixup patches to avoid burdening you with a round-trip.

  Daniel