Skip to content
This repository has been archived by the owner on Jan 7, 2020. It is now read-only.

Improve function naming (eg: newPublic is confusing) #342

Open
1 task
joshuef opened this issue Jan 18, 2019 · 3 comments
Open
1 task

Improve function naming (eg: newPublic is confusing) #342

joshuef opened this issue Jan 18, 2019 · 3 comments

Comments

@joshuef
Copy link
Contributor

joshuef commented Jan 18, 2019

Lets check our method naming to be as clear as possible.

eg: newPublic can actually retrieve a data existing, but that's suuuuper not clear.

There are for sure other confusing naming conventiones, so I'd propose we make a list and then clarify them:

  • newPublic -> addOrGetNewPublic ?
@happybeing
Copy link

One that springs to mind is SAFE NFS open() which is also used to create a new file. It would be clearer if we had a separate create() function.

@b-zee
Copy link
Contributor

b-zee commented Jan 18, 2019

Since I've been digging into the client library APIs I understand much more of what is happening under the hood. safe_app_nodejs mixes up the MutableData and MDataInfo data structures into one class. new_public is about the MDataInfo.

Changing newPublic to addOrGetNewPublic is not just about the name; that will change what the function does. What if I'm sure no MD exists (e.g. hash of something random)? I don't want it to check the network to see if it exists, that costs time.

Perhaps this is about how the API is structured and how it relates to the Rust API.

@oetyng
Copy link

oetyng commented Jan 18, 2019

When it comes to Add / Create / Insert, I would strive for exclusive use of one of them throughout all, for the exact same operation.
Then GetOrAdd, CreateIfNotExists, Getsert (from Upsert, i.e. UpdateOrInsert), and so on would be picked based on that.

Regarding what @b-zee mentions:
Many API:s provide one method for the GetOrAdd case, and one for the Add case, which is expected to throw (or return error result) when already exists.

About the current name change suggestion (newPublic => addOrGetNewPublic), I would mabye think that New is redundant. It depends a bit, because Add is not as clearly evident that it is new, as for example Create. So as an example, GetOrCreatePublic would suffice IMO, if Create was used.

Edit: I use PascalCase because I look at this from the C# API perspective. The question spans the various language bindings, and preferably would be reflected in the Rust layer as well.

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

No branches or pull requests

4 participants