Euphoria
Ticket #714:
load_map() and save_map() inconsistent language constructs
-
Reported by
euphoric
Sep 10, 2011
No other map proc/func is appended by "_map," and there are lots of them.
For consistency, these functions should be renamed load() and save().
Details
1. Comment by DerekParnell
Sep 10, 2011
The reason these have the "_map" in them is because they are not loading/saving anything else but a map. It is possible that a future routine in the map library might load/save something other than a map.
2. Comment by euphoric
Sep 10, 2011
Then those future routines can have an appended "[load|save]_whatever." But the basic map loading and saving routines don't need it.
3. Comment by mattlewis
Sep 10, 2011
meh
4. Comment by jimcbrown
Sep 10, 2011
I'm with mattlewis on this one.
5. Comment by jeremy
Sep 10, 2011
With XML possibly coming to Euphoria w/4.1, there could be:
map:save_xml(my_map, "abc.xml")
my_map = map:load_xml("abc.xml")
Dunno, just a thought. Is it clear from
map:save(my_map, filename)
map:save_xml(my_map, filename)
what is actually happening? What about from
map:save_map(my_map, filename)
map:save_xml(my_map, filename)
?? Remember, a perfect name is concise yet self documenting. Sure, finer details may need a reference to the docs but one should be able to read code and understand the basic jist of things w/o referring to the docs.
I guess to me, both "save_map" and "save_xml" are saving a MAP. Thus, save_map is a bit of a misnomer? What is happening is they are saving a map to two different file formats/types. Who knows what others may come up with, such as save_json, load_json. Again, they are saving/loading a map, just to/from a different file type.
6. Comment by euphoric
Sep 10, 2011
Jeremy, we already have a "type_" parameter, which currently can be set to SM_TXT or SM_RAW (and defaults to SM_TXT). This can be expanded to SM_XML, SM_JSON, to cover all those cases and more.
Derek, could you expand on what the map library could save that is not a map?
Otherwise, consistency dictates this be changed to map:save().
7. Comment by jeremy
Sep 10, 2011
@euphoric: Hm, I didn't realize that. About extending it to support xml/json/xyz_abc... Not sure it would be a good plan to keep on growing that function. Maybe those formats should actually manage that. Maybe that's a better idea to my obviously incorrect example.
include std/xml/expat.e
include std/xml/map.e
include std/map.e
map m = xml_map:load("abc.xml")
Jeremy
8. Comment by euphoric
Sep 10, 2011
I guess it could be done as an expansion/plugin of sorts:
include std/map.e -- standard map:save() functionality with SM_TXT and SM_RAW formats
include std/xml/map.e -- but I want to use the XML format to save, so I need this "map plugin"
include std/json/map.e -- and maybe I want to use JSON... :)
map:save( mymap, fname ) -- default map save, uses routine_id of built-in SM_TXT map writer
map:save( mymap, fname, SM_XML ) -- SM_XML is defined in std/xml/map.e and is the routine_id of the XML map writer function
map:save( mymap, fname, SM_JSON ) -- SM_JSON is defined in std/json/map.e and is the routine_id of the JSON map writer function
-- or, maybe better, a generic routine:
map:save( mymap, fname, xml:MAP_WRITER )
-- or just
map:save( mymap, fname, json:WRITER )
So, maybe the best way for expansion is to use a routine_id that gets passed to map:save() to identify what function should be used to save the map. Now we can let the format libraries handle formatting for the map library.
9. Comment by DerekParnell
Sep 11, 2011
Just some quick ideas ...
save_data(mymap, ...)
save_keys(mymap, ...)
save_meta(mymap, ...)
save_all( {map1, map2, map3, ...}, ...)
GitHub