search

yonghah / esri2sf

Scrape features from ArcGIS Server REST API and create simple features dataframe
Other
137 stars 37 forks source link

Standardize function naming convention #42

Open jacpete opened 2 years ago

jacpete commented 2 years ago

This issue is a place to discuss the standardization of function names within the package so future development will have some standards to work off of.

This was rightfully brought up by @elipousson in https://github.com/yonghah/esri2sf/issues/39#issue-1065599692 with:

  • The original esri2sf and esri2df functions used the convention of all lowercase function names. I noticed that more recent additions use a camel case convention that matches the conventions of ArcGIS web services. I stuck with the latter in this draft but wasn't sure if all lowercase may be preferred.

And I responded with https://github.com/yonghah/esri2sf/issues/39#issuecomment-1007890651:

This is definitely been influenced by my personal taste. I tend to mix camelCase with underscores being used for separating parts of a name. For example the esriUrl_ prefixed functions all deal with the same type of data (ESRI url strings) and then the actual function name is base of the full function name (esriUrl_isValid, esriUrl_parseUrl). I know this doesn't agree with the original esri2sf function that you rightly noted was all lowercase, but it does follow the camelCase of the functions in the zzz.R file that were there before I started adding stuff to this repo. Totally up for debate on what we decide and use as standards moving forward, just want to make sure we don't change too much of pieces that already exist as exported functions so we don't break peoples existing code (we can create aliases and use deprecation warnings if we do end up changing an exported function: see here for an example). My current (biased) suggestion is to use camelCase for new functions except for the core original function esri2sf and its direct counterpart esri2df. My main reason for avoiding lowercase names is readability but the '2' in these function names solves that issue. @yonghah feel free to chime in on any thoughts you have here as well. I am happy to keep adding functionality and fixing bugs in the package but when it comes to stylistic decisions I want to make sure your'e on board as well.

I wanted to make this it's own prominent issue for us all to discuss and decide on the best path forward.