| 2015-03-23 16:45:32 |
Chris Coulson |
description |
I've tried to rearrange code to make it clearer which code belongs in, say, WebViewAdapter in qt/core/glue and WebView in qt/core/browser, but the code split still isn't really all that clear (even to me). In addition to that, there's other problems:
- There's a lot of implementation details exposed between the 2 directories, using private methods and friend statements which all seems a bit wrong.
- Using the WebView case again, there may be a legitimate reason for another class in Oxide to do OxideQQuickWebViewPrivate::get() in order to access a subset of methods on OxideQQuickWebViewPrivate, but it also exposes the entire API surface of WebViewAdapter as its methods have to be public in order to be accessible from OxideQQuickWebView (unless we make them protected and use a friend statement).
I think we should refactor it all to do this instead (using webview as an example):
(sorry if this is not clear)
class OxideQQuickWebView : public QQuickItem {
public:
...
<public API>
...
private:
QScopedPointer<OxideQQuickWebViewPrivate> d_ptr;
};
class OxideQQuickWebViewPrivate : public oxide::qt::WebViewProxyClient {
public:
static OxideQQuickWebViewPrivate* get(OxideQQuickWebView* q);
...
<methods available to other code in qt/quick/api>
...
private:
...
<implementation of WebViewProxyClient>
...
QScopedPointer<oxide::qt::WebViewProxy> proxy_;
};
WebViewProxy and WebViewProxyClient are abstract interfaces in qt/core/glue, although WebViewProxy has a static create() function.
In qt/core/browser:
WebView : public WebViewProxy {
public:
...
<methods available to code in qt/core/browser>
...
private:
...
<implementation of WebViewProxy>
...
WebViewProxyClient* client_;
}; |
I've tried to rearrange things to make it clear how code should be split between, say, WebViewAdapter in qt/core/glue and WebView in qt/core/browser, but the code split still isn't really all that clear (even to me). In addition to that, there's other problems:
- There's a lot of implementation details exposed between the 2 directories, using private methods and friend statements which all seems a bit wrong.
- Using the WebView case again, there may be a legitimate reason for another class in Oxide to do OxideQQuickWebViewPrivate::get() in order to access a subset of methods on OxideQQuickWebViewPrivate, but it also exposes the entire API surface of WebViewAdapter as its methods have to be public in order to be accessible from OxideQQuickWebView (unless we make them protected and use a friend statement).
I think we should refactor it all to do this instead (using webview as an example):
(sorry if this is not clear)
class OxideQQuickWebView : public QQuickItem {
public:
...
<public API>
...
private:
QScopedPointer<OxideQQuickWebViewPrivate> d_ptr;
};
class OxideQQuickWebViewPrivate : public oxide::qt::WebViewProxyClient {
public:
static OxideQQuickWebViewPrivate* get(OxideQQuickWebView* q);
...
<methods available to other code in qt/quick/api>
...
private:
...
<implementation of WebViewProxyClient>
...
QScopedPointer<oxide::qt::WebViewProxy> proxy_;
};
WebViewProxy and WebViewProxyClient are abstract interfaces in qt/core/glue, although WebViewProxy has a static create() function.
In qt/core/browser:
WebView : public WebViewProxy {
public:
...
<methods available to code in qt/core/browser>
...
private:
...
<implementation of WebViewProxy>
...
WebViewProxyClient* client_;
};
This fixes the above problems (we don't have to expose implementation details between qt/core/browser and qt/core/glue, the WebViewProxy instance is private to OxideQQuickWebViewPrivate and its API isn't exposed to classes that call OxideQQuickWebViewPrivate::get(), and developers don't need to spend time trying to figure out which directory to put code in to) |
|
