Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid general function names #3

Open
BPJandree opened this issue Oct 29, 2020 · 1 comment
Open

avoid general function names #3

BPJandree opened this issue Oct 29, 2020 · 1 comment

Comments

@BPJandree
Copy link

We may want to rethink some of the names.

I think it would be better to use something slightly less general names than add for the main utility I introduced in #2 . For example publish could work.

Also, the function datasets shares a name with the base R datasets package and could be renamed.

I could also image that future users might work on creating R packages and pushing them with nadar. In this cease, there is already a function create in the the standard devtools package which creates R packages and so we may want to avoid using the same function naming.

Since the add function I introduce in #2 always wraps around this create function, we may simply move my suggestions and merge them into the create function and thus of working with create and add just work with a single publish function.

There may be some other examples, I have not yet looked at everything. A careful review would be good.

@thijsbenschop
Copy link
Collaborator

thijsbenschop commented Nov 2, 2020

Thanks @BPJandree for starting the discussion on function names. Taking into account our discussion last week, I wanted to make a first proposal incorporating the suggestions made.

Proposal for grouping and naming of functions
(new functions in bold, replaced names in strikethrough)

globals.R -> api_functions.R
get_api_key
get_api_url
get_verbose
set_api_key
set_api_url
set_verbose
set_api() set key, url and verbose, wrapper for other three functions

datasets.R -> thumbnail.R
thumbnail_delete # delete thumbnail from study
thumbnail_upload thumbnail_add # upload picture as thumbnail
utils.R -> thumbnail.R
capture_pdf_cover # wondering whether we can drop this function as it’s a straightforward operation in R and is not directly related to NADA, can also be added to aux_funtions.R

catalog_entries.R
catalog_list # list all entries in catalog
catalog_search #search catalog to find entry/entries by search term
entry_info # to list info of an entry from catalog_list
catalog_delete entry_delete() # delete single study by providing ID
catalog_find_by_id entry_find_id() # find study by id (for what purpose do we need this one?)
catalog_find_by_idno entry_find_idno() # find study by idno
replace_idno entry_replace_idno # replace old idno by new idno

datasets.R
datasets # identical to catalog_list

utils.R -> aux_functions.R
nada_http_get
is_valid_url
nada_http_get #why do we need this function? Is it just to test the API?

globals.R -> nadar_globals.R
#set globals

external_resources.R
external_resources_list # list external resources for entry
external_resources_add # add new resource to study
external_resources_upload external_resources_upload_file # Upload file/content for external resource
external_resources_download_file # Donwload file/content for external resource
external_resources_import external_resources_import_rdf # Import/upload rdf file

create create_entry

For the *_add functions, I would suggest to keep both a generic create_entry function that uses a type argument as discussed in #2. For the *_add function, we could consider swapping add and the type, e.g. add_script instead of script_add.

The name publish may be misleading, as there is an argument publish in this function.

document_add
geospatial_add
image_add
script_add
table_add
timeseries_add
timeseries_database_add

microdata_add
import_ddi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants