Re: [MERGE] Initial netfilter mark patch for comment

On Fri, 2010-08-13 at 18:19 -0600, Alex Rousskov wrote:
> On 08/11/2010 03:25 PM, Andrew Beverley wrote:
>
> > I've moved these, as well as most of the other QOS functions, into
> > Ip::Qos. I have also removed the QosConfig namespace, as it didn't seem
> > to fit with all these additional functions.
>
> * A patch preamble with the proposed commit message would be nice.
>
> * I am not sure what Qos class is. It is not documented. If it is a "QOS
> configuration" class, I understand why we have a global instance of it,
> but the original QosConfig name seems better in that case. If it is not
> a configuration class, then I am not sure why we have a global instance
> of it. And the QosConfig file name does not seem to match the class name
> any more.
>
> Perhaps we need two classes, one for configuration and one for
> manipulation? Or a Qos namespace with a configuration class and global
> manipulation functions? The latter seems more likely.
>

I was trying to copy the approach used by the Interceptor class, which
has its configuration variables and methods in one class, with a global
instance of it. I thought that it would keep things nice and tidy by
having all qos aspects in one single class (with the configuration
variables within private space).

I'm happy to go with whatever approach you want though, and can separate
the config aspects back out from the functions.

Before I go ahead and do this, I want to check which option I should go
for. As Alex says, the options are:

1)
- Reinstate the QosConfig class with the configuration variables as
public members
- Create a new class (QosFunctions?) with the relevant functions in
- Create one global instance of each class, or alternatively create a
new instance of the functions class each time the functions need to be
accessed (is the latter inefficient?)

2)
- Reinstate the QosConfig class with the configuration variables as
public members
- Create the functions as global functions in the namespace

3)
- Keep everything in one class :-)

If going with option 2, the functions can no longer be const (which most
of them currently are).

Thanks,

Andy
Received on Fri Aug 20 2010 - 17:06:45 MDT