[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: koffice
From:       Inge Wallin <inge () lysator ! liu ! se>
Date:       2009-10-31 16:55:58
Message-ID: 200910311755.58997.inge () lysator ! liu ! se
[Download RAW message or body]

On Saturday 31 October 2009 17:25:23 Thorsten Zachmann wrote:

> There is a lot of code duplication in there (the above part is repeated 4
> times) . How about creating a function to remove the code duplication?

There is indeed a lot of code duplication in there.  Creating a function would 
work, but I have an even better plan that I was going to investigate and then 
send a mail to koffice-devel about this evening.

If you look at the details, there are also a lot of subtle differences between 
the four cases, but I guess those can be overcome with some creative use of 
static arrays.

The proposal is about that as far as I can see, there is no enum Edge {left, 
top, right, bottom}, which would be very useful to have.  I see this in many 
places in KOffice that they are redefined over and over again.  Not many 
places allow data to be fetched with reference to which edge it is applied to.

When I asked in #koffice about whether to design the border API like this:

BorderStyle border.getStyle(Edge edge);

or like this:

BorderStyle border.getLeftStyle();
BorderStyle border.getTopStyle();
BorderStyle border.getRightStyle();
BorderStyle border.getBottomStyle();

...the voices where overwhelmingly to using the second way.  I don't like that 
very much, but I've gotten so much flak lately for not following the community 
that I relented, and it wasn't very important for me.

But I would really like is to introduce a common way for all koffice to 
enumerate edges, and have that used in all places.  I think we could save a 
lot of code and complexity.

Update:
A more creative grep revealed that there is actually something like this in 
KoTableCellStyle.  The enum is called Side instead of Edge, but that's a 
trivial difference.

I propose that we introduce the following somewhere in kobase (which should 
perhaps be called koutils or something since there are no base classes in 
there but utilities):

enum Side {
  LeftSide,
  TopSide,
  RightSide,
  BottomSide
};
const Side sides[] = {LeftSide, TopSide, RightSide, BottomSide};

or Edge instead of Side if you prefer that.

	-Inge








_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel
[prev in list] [next in list] [prev in thread] [next in thread]