Opened 9 years ago

Closed 9 years ago

#27 closed defect (fixed)

LayerRecord signature should look like Record signature

Reported by: tschaub Owned by:
Priority: blocker Milestone:
Component: Version:
Keywords: Cc:
State: Needs more work


I think it would be useful for the LayerRecord constructor to look the same as the regular Record constructor (data, id). This would make it easier for a reader to be able to take either recordType.

Change History (4)

comment:1 Changed 9 years ago by ahocevar

Not sure about this. Right now, the layer records and layers sync easily because equals Why would we want a specialized reader to create generic records? I'm open, however, to change the first argument to become an object ({layer: foo}), but this one would always need to have a layer property. Then we could get rid of the 2nd argument and just have*Object*/ data)

The only thing I fear is that ids that the reader sets will be ignored then. But again, why would we want to create generic records with a specialized reader?

comment:2 Changed 9 years ago by ahocevar

Thinking about it a bit more: If the id thing is not important, then the signature could indeed be made the same as in Ext.Record. But in that case, syncing layers between map and store will be more expensive in the LayerStore. Which may not be a big problem. Patches welcome :-)

comment:3 Changed 9 years ago by elemoine

Guys, I created a ticket and a patch for that yesterday, see #25. With my patch the LayerRecord constructor has the same signature as the regular Record constructor, and creating layer records from layers is done with a LayerReader. The FeatureReader and LayerReader actually look very similar, I think we could factorize code a bit here, but this is the subject of another ticket.

comment:4 Changed 9 years ago by elemoine

  • Resolution set to fixed
  • Status changed from new to closed

(In [260]) add LayerReader, r=ahocevar (closes #25, #27)

Note: See TracTickets for help on using tickets.