[jira] [Comment Edited] (S2GRAPH-170) Create Interface for S2Edge/S2Vertex/S2Graph.

["Daewon Jeong (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/S2GRAPH-170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16240094#comment-16240094 ] 

Daewon Jeong edited comment on S2GRAPH-170 at 11/6/17 10:09 AM:
----------------------------------------------------------------

I reviewed the above draft.

I agree to proceed with refactoring to clarify the interface.
However, I have some comments on this refactoring.

# S2 [Vertex, Edge] Like

'SelfType' is used in the 'S2Vertex` and' S2VertexLike 'as there is a bidirectional dependency.

S2VertexLike should be unaware of the existence of S2Vertex because the dependency should flow unidirectionally.

To achieve the above goal, `SelfType` should be removed.

{code:scala}
trait S2EdgeLike extends Edge with GraphElement { this: S2Edge => }
{code}

shoudbe
{code:scala}
trait S2EdgeLike extends Edge with GraphElement { }
{code}


p.s: This refactoring seems to be in gradual improvement for compatibility with existing code (not to break caller).


was (Author: daewon):
I reviewed the above draft.

I agree to proceed with refactoring to clarify the interface.
However, I have some comments on this refactoring.

# S2 [Vertex, Edge] Like

'SelfType' is used in the 'S2Vertex` and' S2VertexLike 'as there is a bidirectional dependency.

S2VertexLike should be unaware of the existence of S2Vertex because the dependency should flow unidirectionally.

To achieve the above goal, `SelfType` should be removed.

p.s: This refactoring seems to be in gradual improvement for compatibility with existing code (not to break caller).

> Create Interface for S2Edge/S2Vertex/S2Graph.
> ---------------------------------------------
>
>                 Key: S2GRAPH-170
>                 URL: https://issues.apache.org/jira/browse/S2GRAPH-170
>             Project: S2Graph
>          Issue Type: Improvement
>    Affects Versions: 0.2.1
>            Reporter: DOYUNG YOON
>             Fix For: 0.2.1
>
>   Original Estimate: 504h
>  Remaining Estimate: 504h
>
> h2. Problem Statement
> S2Graph’s entire code base is dependent on S2Edge/S2Vertex/S2Graph class. Even though lots of code touch theses two class, there is no interface defined currently. This means lots of code is interact with these class in different way all by their own way, and this make extremely hard to make any change on these two classes.
> For example, I was working on S2GRAPH-80 to provide java client, and there are too many places to be changed since all theses places use concrete implementation class S2Edge/S2Vertex/S2Graph, not the interfaces. Not just for S2GRAPH-80, but any other issues that need to change theses classes would benefit by communicating through interface.
> h2. Suggestion
> Define interface and change code base to communicate through theses interfaces.
> # Create interface S2Edge/S2Vertex/S2Graph that implement Tinkerpop’s Edge/Vertex/Graph interface 
> # Extract tinkerpop interface related implementations
> # Change caller of S2Edge/S2Vertex/S2Graph to call interface rather than concrete implementation.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)