OOP question: deduplicating code across two subclasses

[Scott Kostyshak]

On Sun, Dec 11, 2022 at 12:31:18AM +0100, Thibaut Cuvelier wrote:
> 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.

I see, thanks for explaining this idea. I can see the advantage: it
would make it very clear which functions use which parameters.

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/d143ed36/attachment.asc>