diffix / explorer

Tool to automatically explore and generate stats on data anonymized using Diffix
MIT License
2 stars 1 forks source link

sql names quoting take 3 plus related cleanup #238

Closed AndreiBozantan closed 4 years ago

AndreiBozantan commented 4 years ago

Here is a list with some of the changes:

AndreiBozantan commented 4 years ago

queries

This is better. Only thing is that I would prefer if GetQueryStatement was not part of the public API, since it's never called outside the BuildQueryStatement method.

We could have the DQuery interface expose BuildQueryStatement only and instead have GetQueryStatement implemented in an abstract base class DQueryBase (or whatever) as a protected member. The ABC would also provide the default impl for BuildQueryStatement.

Then the query implementations would derive from the DQueryBase instead of implementing the interface directly.

Something like this:

public interface DQuery<TRow>
{
    public string BuildQueryStatement(...);
    public TRow ParseRow(...);
}

public abstract class DQueryBase<TRow> : DQuery<TRow>
{
    public virtual string BuildQueryStatement(...) => GetQueryStatement(...);
    protected abstract string GetQueryStatement(...);
    public TRow ParseRow(...);
}

What do you think?

I am not sure that this will work. There are some query implementations (e.g. Min) that implement the DQuery interface multiple times.

AndreiBozantan commented 4 years ago

queries

This is better. Only thing is that I would prefer if GetQueryStatement was not part of the public API, since it's never called outside the BuildQueryStatement method. We could have the DQuery interface expose BuildQueryStatement only and instead have GetQueryStatement implemented in an abstract base class DQueryBase (or whatever) as a protected member. The ABC would also provide the default impl for BuildQueryStatement. Then the query implementations would derive from the DQueryBase instead of implementing the interface directly. Something like this:

public interface DQuery<TRow>
{
    public string BuildQueryStatement(...);
    public TRow ParseRow(...);
}

public abstract class DQueryBase<TRow> : DQuery<TRow>
{
    public virtual string BuildQueryStatement(...) => GetQueryStatement(...);
    protected abstract string GetQueryStatement(...);
    public TRow ParseRow(...);
}

What do you think?

I am not sure that this will work. There are some query implementations (e.g. Min) that implement the DQuery interface multiple times.

I thought a little bit more about it, and I think that we can use both deriving from ABC, in order to implement the BuildQueryStatement and implementing the DQuery interface for ParseRow multiple times. I will do the changes.