From: Joshua O'M. <jos...@gm...> - 2007-08-30 21:31:29
|
Brian: I think I now have a better idea of what you're proposing...but I think that one or the other of us has not thought it all the way through. :) In particular, are you also proposing that the getVertexAttributes(), etc. methods be abolished? If so, how would users access the attributes of the vertices, etc.? If not, you've got to populate this attribute Map (or Transformer) somehow--and you're back to where you were except that now you've got one Transformer that goes from attributes to vertices, and another that goes from vertices to attributes. Maybe this helps, but I don't see how. In any case, the user has to provide _some_ way of creating vertices; since all of the creation and association of attributes happens invisibly to the user, why does it help to ask the user for a Transformer do the work rather than a Factory? As for your final point, that it would be nice for users to be able to have objects like Address auto-created: granted--although I don't see how your scheme accomplishes that, as it leaves the attributes as String->String maps. The way that I've been thinking about doing this looks something like this: setAttributeParser(String attribute, Transformer<String, ?> parser) which would add (attribute, parser) to an internal Map that would be queried by the edge creation code for each remaining element in the attributeMap; non-null parsers would then be invoked (and null parsers would indicate that you just want the string->string mapping). (Granted, you could provide a single method to which you'd pass a Transformer<String, Transformer<String, ?>> rather than telling the class about each parser individually. But since these parsers would be individual creations anyway, I don't think it would help.) Anyway, thanks for your input; it's good to get a sanity check. Joshua On 8/30/07, Brian de Alwis <bs...@cs...> wrote: > > Not sure what you're getting at re: Factories. The Factories don't > > map anything onto anything; they just generate objects as needed. > > Maybe you meant Maps instead (which is how attributes are currently > > stored)? > > Using a Factory to create a {vertex, edge, graph} requires users > to perform a second step to process those resulting attribute maps. > We could use a Transformer to instead make this a single step. > > Consider Anne-Marie's next step, assuming she was successful in > writing out her graph, and wished to read it in later: > > > I have attributes of basic types (string, etc) and some that I > > have defined (Address - has a toString method). > > Using the current style of implementation, after using GraphMLFile > to load the file, she will need to walk the vertex attributes map > and then update her vertex objects (e.g., parsing the address > attribute into an Address object). > > Under my proposal, she would provide a Transformer whose task is to > return a suitable object and configured using the provided attributes. > > It's perhaps easier to demonstrate what I mean by code: > > public class GraphMLFileRader<V,E> { > > Transformer<Map<String,String>, V> vertexCreator; > Transformer<Map<String,String>, E> edgeCreator; > Transformer<Map<String,String>, G> graphCreator; > > // etc. etc. > > protected V createVertex(Map<String,String> attributeMap) { > if (mGraph == null) { > throw new RuntimeException("Error parsing graph. Graph element must > be specified before node element."); > } > > String idString = attributeMap.remove("id"); > V vertex = vertexCreator.transform(attributeMap); > mGraph.addVertex(vertex); > > if(mLabeller.put(idString, vertex) != null) { > throw new RuntimeException("Ids must be unique"); > } > > return vertex; > } > > > protected E createEdge(Map<String,String> attributeMap) { > if (mGraph == null) { > throw new RuntimeException("Error parsing graph. Graph element must > be specified before edge element."); > } > > String sourceId = attributeMap.remove("source"); > V sourceVertex = > mLabeller.get(sourceId); > > String targetId = attributeMap.remove("target"); > V targetVertex = > mLabeller.get(targetId); > > String direction = attributeMap.remove("directed"); > EdgeType directed; > if (direction == null) > { > // use default_directed > directed = default_directed; > } > else > { > // use specified direction > if (direction.equals("true")) > directed = EdgeType.DIRECTED; > else if (direction.equals("false")) > directed = EdgeType.UNDIRECTED; > else > throw new RuntimeException("Error parsing graph: 'directed' tag > has invalid value: " + direction); > } > E e = edgeCreator.transform(attributeMap); > mGraph.addEdge(e, sourceVertex, targetVertex, directed); > > return e; > } > > protected void createGraph(Map<String,String> attributeMap) { > String edgeDefaultType = attributeMap.remove("edgedefault"); > > // XXX: maybe this should use edgeDefaultType to test > // the type of the graph > mGraph = graphCreator.transform(attributeMap); > > if (edgeDefaultType.equals("directed")) { > default_directed = EdgeType.DIRECTED; > } > else if (edgeDefaultType.equals("undirected")) > { > default_directed = EdgeType.UNDIRECTED; > } > else { > throw new RuntimeException("Error parsing graph. EdgeType default ty > pe not specified."); > } > mLabeller = new HashMap<String,V>(); > //StringLabeller.getLabeller(mGraph); > > } > } > > -- > Brian de Alwis | Software Practices Lab | UBC | http://www.cs.ubc.ca/~bsd/ > "Amusement to an observing mind is study." - Benjamin Disraeli > -- jos...@gm......................www.ics.uci.edu/~jmadden Joshua O'Madadhain: Information Scientist, Musician, Philosopher-At-Tall It's that moment of dawning comprehension that I live for. -- Bill Watterson My opinions are too rational and insightful to be those of any organization. |