OOP question: deduplicating code across two subclasses

[Thibaut Cuvelier]

On Sat, 10 Dec 2022 at 23:12, Scott Kostyshak <skostysh at lyx.org> wrote:

> On Sat, Dec 10, 2022 at 10:36:51PM +0100, Thibaut Cuvelier wrote:
> > On Sat, 10 Dec 2022 at 17:19, Scott Kostyshak <skostysh at lyx.org> wrote:
> >
> > > PreambleModule::editExternal() and LocalLayout::editExternal() share
> > > most of their code. The functions are pretty small, so there's not too
> > > much duplication as a whole. However, I'm still curious. How would I
> > > best deduplicate the code?
> > >
> > > In this case I wonder if a friend function might be the best course of
> > > action, but I'm not confident so I wanted to check in for advice before
> > > pursuing that.
> > >
> > > Does a friend function indeed seem reasonable for this? Does this
> > > violate a guideline of one of the various lineages of OOP philosophy?
> > > Perhaps some guidelines say that every function that accesses
> > > private/public members should be a member function of a superclass?
> > > i.e., in this case I should add a member function to a shared
> superclass
> > > of PreambleModule and LocalLayout?
> > >
> > > I've searched for general guidelines, and indeed found a lot of results
> > > (such as [1] and [2]), but I am unsure how they apply to this specific
> > > case.
> > >
> >
> > My two cents, here, taking a purist point of view (i.e. not pragmatic at
> > all). These two functions are quite long and they mix a lot of things:
> why
> > are you dealing with files in purely UI code? How much would it be doable
> > to split them into multiple, smaller functions (in an anonymous namespace
> > in the .cpp file)? It would also make the code much easier to read,
> because
> > I have no clue about what these functions are doing (maybe it would make
> > sense to encapsulate some of the common code in a class?).
> > Having a friend function wouldn't solve the readability issues, only
> > duplication, but I'm not sure that duplication is the worst here.
> >
> > That's not an OOP answer, but is really OOP the answer to each and every
> > problem? In a sense, my answer is a lot about the SRP
> > <https://en.wikipedia.org/wiki/Single-responsibility_principle>, though.
>
> Thanks, Thibaut. That is indeed interesting and helpful. I am happy to
> take a look at factoring out the code in the functions to make the code
> easier to read.
>
> But unless I'm mistaken, don't the smaller functions still need access
> to the data members? Or you mean to pass the data members as arguments?
> Perhaps your point is that once I make meaningful smaller functions, it
> will make more sense to add them as useful member functions rather than
> friend functions?
>

Yes, that's what I had in mind: free functions, outside of classes, with
parameters (rather than member functions). I believe you can achieve
better-looking code without having too many parameters.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lyx.org/pipermail/lyx-devel/attachments/20221211/a8df81c5/attachment.html>