stcorp / harp

Data harmonization toolset for scientific earth observation data
http://stcorp.github.io/harp/doc/html/index.html
BSD 3-Clause "New" or "Revised" License
55 stars 19 forks source link

duplicate code: hashtable #220

Closed schwehr closed 4 years ago

schwehr commented 4 years ago

Both coda and harp have hashtable.[ch]. Can harp just use hashtable from coda as harp requires coda?

That would remove 440 lines from harp.

diff stcorp_coda/libcoda/hashtable.h stcorp_harp/libharp/hashtable.h
2c2
<  * Copyright (C) 2007-2020 S[&]T, The Netherlands.
---
>  * Copyright (C) 2015-2020 S[&]T, The Netherlands.
35,41d34
< /* *INDENT-OFF* */
< #ifdef __cplusplus
< extern "C"
< {
< #endif
< /* *INDENT-ON* */
< 
50,55c43,48
< #define hashtable_add_name coda_hashtable_add_name
< #define hashtable_delete coda_hashtable_delete
< #define hashtable_get_index_from_name coda_hashtable_get_index_from_name
< #define hashtable_get_index_from_name_n coda_hashtable_get_index_from_name_n
< #define hashtable_insert_name coda_hashtable_insert_name
< #define hashtable_new coda_hashtable_new
---
> #define hashtable_add_name harp_hashtable_add_name
> #define hashtable_delete harp_hashtable_delete
> #define hashtable_get_index_from_name harp_hashtable_get_index_from_name
> #define hashtable_get_index_from_name_n harp_hashtable_get_index_from_name_n
> #define hashtable_insert_name harp_hashtable_insert_name
> #define hashtable_new harp_hashtable_new
66,70d58
< /* *INDENT-OFF* */
< #ifdef __cplusplus
< }
< #endif
< 
72d59
< /* *INDENT-ON* */

diff stcorp_coda/libcoda/hashtable.c stcorp_harp/libharp/hashtable.c
2c2
<  * Copyright (C) 2007-2020 S[&]T, The Netherlands.
---
>  * Copyright (C) 2015-2020 S[&]T, The Netherlands.
svniemeijer commented 4 years ago

'hashtable' is not a public interface of CODA. And HARP can not depend on private interfaces of CODA. It will break things in terms of shared library versioning (how HARP links to a specific CODA shared library version for ABI compatibility), if we ever make a change in the hashtable code in CODA (since it won't be marked as a change in the public interface). Also, some platforms (e.g. Windows) don't like it if you try to use symbols that are not explicitly exported.

So the short answer is no.

The only proper way to reuse the CODA implementation of hashtable would be to make its api part of the public CODA api (which is not something we plan to do) or to turn the hashtable part into its own code base (which is overkill for how small the code is). So the current approach of having the implementation available twice is the most optimal in terms of maintenance and clear separation of the CODA and HARP code bases.