Repository: zeppelin
Updated Branches:
  refs/heads/master f12bc26bb -> 4beeae844


[ZEPPELIN-2493] Visualization class should throw an error if an abstract method 
is not implemented

### What is this PR for?
`Visualization` class should throw an error if an abstract method is not 
implemented just like 
[`SpellBase`](https://github.com/apache/zeppelin/blob/master/zeppelin-web/src/app/spell/spell-base.js#L25)
 does.

And I added some comments. In my opinion, though `Visualization` has [a good 
example](http://zeppelin.apache.org/docs/0.8.0-SNAPSHOT/development/writingzeppelinvisualization.html),
 it needs an API documentation page (`Spell` as well). `Visualization` class 
has methods which can be overridden (e.g. `refresh`, `destroy`, etc.), but the 
example doesn't mention them. Therefore, users don't know what and how to 
implement easily!

### What type of PR is it?
Improvement

### What is the Jira issue?
[ZEPPELIN-2493](https://issues.apache.org/jira/browse/ZEPPELIN-2493)

### How should this be tested?
* Inherit `Visualization`
* Don't implement any abstract method
* Run `getTransformation()` and `render()`
* Check they print appropriate errors

### Questions:
* Does the licenses files need update? NO
* Is there breaking changes for older versions? NO
* Does this needs documentation? NO

Author: Jun Kim <[email protected]>

Closes #2317 from tae-jun/viz-abstract-error and squashes the following commits:

3fec6d3 [Jun Kim] Throw error if Visualization class is not implemented and 
make doc more concrete
d994b8a [Jun Kim] Throw error if an abstract method is not implemented


Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo
Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/4beeae84
Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/4beeae84
Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/4beeae84

Branch: refs/heads/master
Commit: 4beeae84413c4dca01674f3f2f991a7fc08c0097
Parents: f12bc26
Author: Jun Kim <[email protected]>
Authored: Thu May 4 14:34:21 2017 +0900
Committer: Lee moon soo <[email protected]>
Committed: Tue May 23 10:26:26 2017 +0900

----------------------------------------------------------------------
 .../src/app/visualization/visualization.js      | 25 ++++++++++++--------
 1 file changed, 15 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zeppelin/blob/4beeae84/zeppelin-web/src/app/visualization/visualization.js
----------------------------------------------------------------------
diff --git a/zeppelin-web/src/app/visualization/visualization.js 
b/zeppelin-web/src/app/visualization/visualization.js
index ec89882..82704e3 100644
--- a/zeppelin-web/src/app/visualization/visualization.js
+++ b/zeppelin-web/src/app/visualization/visualization.js
@@ -13,7 +13,7 @@
  */
 
 /**
- * Base class for visualization
+ * Base class for visualization.
  */
 export default class Visualization {
   constructor (targetEl, config) {
@@ -25,17 +25,22 @@ export default class Visualization {
   }
 
   /**
-   * get transformation
+   * Get transformation.
+   * @abstract
+   * @return {Transformation}
    */
   getTransformation () {
     // override this
+    throw new TypeError('Visualization.getTransformation() should be 
overrided')
   }
 
   /**
-   * Method will be invoked when data or configuration changed
+   * Method will be invoked when data or configuration changed.
+   * @abstract
    */
   render (tableData) {
     // override this
+    throw new TypeError('Visualization.render() should be overrided')
   }
 
   /**
@@ -46,7 +51,7 @@ export default class Visualization {
   }
 
   /**
-   * method will be invoked when visualization need to be destroyed.
+   * Method will be invoked when visualization need to be destroyed.
    * Don't need to destroy this.targetEl.
    */
   destroy () {
@@ -64,7 +69,7 @@ export default class Visualization {
   }
 
   /**
-   * Activate. invoked when visualization is selected
+   * Activate. Invoked when visualization is selected.
    */
   activate () {
     if (!this._active || this._dirty) {
@@ -75,21 +80,21 @@ export default class Visualization {
   }
 
   /**
-   * Activate. invoked when visualization is de selected
+   * Deactivate. Invoked when visualization is de selected.
    */
   deactivate () {
     this._active = false
   }
 
   /**
-   * Is active
+   * Is active.
    */
   isActive () {
     return this._active
   }
 
   /**
-   * When window or paragraph is resized
+   * When window or paragraph is resized.
    */
   resize () {
     if (this.isActive()) {
@@ -100,7 +105,7 @@ export default class Visualization {
   }
 
   /**
-   * Set new config
+   * Set new config.
    */
   setConfig (config) {
     this.config = config
@@ -119,7 +124,7 @@ export default class Visualization {
   }
 
   /**
-   * render setting
+   * Render setting.
    */
   renderSetting (targetEl) {
     let setting = this.getSetting()

Reply via email to