Opened 7 years ago

Closed 7 years ago

#1003 closed task (fixed)

Refactor code to avoid direct use of textfile map code for bookmarks/last destinations

Reported by: sleske Owned by: KaZeR
Priority: minor Milestone:
Component: core Version: git master
Severity: Keywords:
Cc:

Description

As pointed out in the comments to #980, at the moment, the code for bookmarks/last destinations is breaking code consistency.

navit.c directly #includes map/textfile/textfile.h from outside of map/textfile/ (see FIXME there). However, the textfile code (like all the code under map/ ) is meant to be separate from the main code, and should only be accessed via the standard map plugin interface.

The underlying problem is that the bookmark/last destination code (bookmarks_append_coord etc. in bookmarks.c) contains code to output the bookmarks.txt and destination.txt files. This code has to output the files such that they can be read by the textfile map handling code - thus there is an implicit dependency on the textfile format.

Normally, map driver interaction should be done through existing map interface. However, the existing map interface does not handle writing, thus the bookmark code has to do it itself.

This dependency should somehow be resolved by appropriate refactoring/restructuring.

Change History (5)

comment:1 Changed 7 years ago by sleske

Ideas for resolving this dependency:

  • Add writing methods to the map plugin interface, and move the writing code from bookmarks.c to textfile.c. Clean solution, but may be over-engineering (because textfiles are the only map type where write support is needed, so the map driver interface would become more complex because of a single map type).

or

  • Move most of the current textfile code from map/textfile to a new folder (let's say textfile/ ). Add writing methods under textfile/*. Then both the bookmark handling code and the map driver under map/textfile can use the code under textfile/* . That way the map driver code does not need to be used other than through its interface, but the write methods are still accessible.

I'll think about this...

Last edited 7 years ago by sleske (previous) (diff)

comment:2 Changed 7 years ago by tryagain

I'd vote for the first solution, i.e. adding write support to map plugin interface.

That new interface can also be used when storing tracks, waypoints etc. We probably have similar issue with csv file writing feature.

comment:3 Changed 7 years ago by sleske

Hm, I'm sceptical about adding write support to the map interface. As far as I can see, write support is only needed (and probably only makes sense) for the textfile format, not for any of the other formats, so it seems confusing to add it to the map interface.

Anyway, good point about storing tracks & waypoints. They should probably be refactored as well.

comment:4 Changed 7 years ago by tegzed

Hello,

Actually we already have support for adding items to maps and modifying items in the map and item interfaces, so I think textfile should implement these operations also. See functions map_rect_create_item,item_coord_set,item_attr_set the supposed usage can be seen in functions navit_cmd_map_add_curr_pos, navit_cmd_map_item_set_attr The csv driver implements the operations of this interface that are needed for adding items and attributes. Deleting items and attributes is not yet supported in the csv driver.

comment:5 Changed 7 years ago by sleske

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

This was fixed in r4939, by moving the #define from textfile.h to navit.c.

Note: See TracTickets for help on using tickets.