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

Avery Ching commented on GIRAPH-36:
-----------------------------------

Tried out your stuff Jake.  Unittests passed for me.  I took a quick look at it 
and was happy to see the new input format style interfaces (nextVertex() and 
getCurrentVertex()).  Also very cool to see LongDoubleFloatDoubleVertex in 
action as a proof-of-concept non-Vertex implementation.  Couple of 
questions/comments.

1)  I would personally prefer that GraphState not be exposed to 
developers/users from VertexReader.  We can always set the GraphState from the 
infrastructure after the vertices have been read.  

2)  Does VertexReader really need the message type M?  There aren't any 
messages at that point and complicates things a bit.

3)  I know it's not ready for primetime as you mentioned, but while we 
currently have two styles of formatting, hopefully we can at least keep files 
consistent until one of us fixes everything to the new convention.
                
> Ensure that subclassing BasicVertex is possible by user apps
> ------------------------------------------------------------
>
>                 Key: GIRAPH-36
>                 URL: https://issues.apache.org/jira/browse/GIRAPH-36
>             Project: Giraph
>          Issue Type: Improvement
>          Components: graph
>    Affects Versions: 0.70.0
>            Reporter: Jake Mannix
>            Assignee: Jake Mannix
>            Priority: Blocker
>             Fix For: 0.70.0
>
>         Attachments: GIRAPH-36.diff
>
>
> Original assumptions in Giraph were that all users would subclass Vertex 
> (which extended MutableVertex extended BasicVertex).  Classes which wish to 
> have application specific data structures (ie. not a TreeMap<I, Edge<I,E>>) 
> may need to extend either MutableVertex or BasicVertex.  Unfortunately 
> VertexRange extends ArrayList<Vertex>, and there are other places where the 
> assumption is that vertex classes are either Vertex, or at least 
> MutableVertex.
> Let's make sure the internal APIs allow for BasicVertex to be the base class.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to