OOP question: deduplicating code across two subclasses

[Scott Kostyshak]

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?

Scott
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.lyx.org/pipermail/lyx-devel/attachments/20221210/b246df1d/attachment.asc>