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

create plug-in interfaces for http-request, http-response, http-headers #37

Open
Tracked by #35
jimkring opened this issue Jan 31, 2024 · 7 comments
Open
Tracked by #35

Comments

@jimkring
Copy link
Contributor

No description provided.

@francois-normandin
Copy link
Collaborator

Creation of a http-client library to be used as the interfaces for REST and specific http clients.
Essentially, this part from discussion in PR #31

http-client

@francois-normandin
Copy link
Collaborator

francois-normandin commented Feb 1, 2024

@jimkring @jyoung8711
Naming convention... so we start on a good footing.

Context:
I sometimes put hyphens in my class names when I'm unsure where it's going to end up... but that's not any standard that I know and I certainly do a lot of renaming when the interfaces coalesce. Hyphens are especially frowned upon in Python where underscores are more traditional (some popular dictionary serialization libraries actually do not work with hyphens, which makes my python colleagues mad) . dotNet is more of a mix of camel and pascal case conventions.
Before I start pushing classes and interfaces around... I'll wait for comments.

Some possible variants:

  • http-client, http-client-basic
  • http-client, basic-http-client
  • http_client, http_client_basic (snake_case)
  • httpClient, httpClient.basic (camelCase)
  • HttpClient, HttpClient.Basic (PascalCase)
  • httpClient (camel interface), BasicHttpClient (pascal concrete classes)
  • something else to propose?

Now that I`ve listed a few, my preference would be for :
Interfaces (and abstract classes) in camelCase, and concrete implementations in PascalCase.
That would result in

  • httpClient
  • BasicHttpClient or HttpClient.Basic

Dot Extension for class hierarchy? BasicHttpClient vs HttpClient.Basic

Definitely nice for maintainers to follow the trace of what inherits from what. Easy to sort on disk when dot notation is used.
I tend to follow the dot notation for lvlibs, so that PPLs can be sorted based on dependencies.

Classes too? It makes for longer names and users of the library might not care much?

image

@jimkring
Copy link
Contributor Author

jimkring commented Feb 1, 2024

Great ideas and an important discussion!

Here are some thoughts:

  • I do a fair amount of python programming, and I find that my class and method/function conventions have moved in the direction of following pythons naming conventions (ClassName.lvclass, method_name.vi)
  • I tend to avoid including parent/ancestor classes in the filenames of child classes (Stepper.Motor.Device.lvclass), since inheritance can change and renaming classes has historically been frowned upon and continues to be a bit of a pain.
  • Using camelCase for interfaces and PascalCase for classes is a clever idea. It certainly gives a nice clue for developers. Sidenote: I believe that NI would have used a different file extension, but it would’ve taken a significant amount of effort to add a new file type, so it was easier to just use *.lvclass.
  • After thinking through that last point, it makes me wonder if using PascalCase.interface.lvclass (or some abbreviated form) could work.
  • Python’s naming conventions for acronyms is to use uppercase for the whole acronym, like HTTPClient.

With all of that said, I also should mention that, for some reason, my brain does not easily parse the words in camelCase or PascalCase, especially when I’m looking at a bunch of files on disk. However, my brain may work differently from most :)

I’ll continue to think about this, and I’ll look for other resources on what standards others are currently adopting…

@jyoung8711
Copy link
Collaborator

jyoung8711 commented Feb 1, 2024

Good / Interesting Discussion on here
(this also feels like a good conversation to have at something like GLA as a community topic in general).

My feedback is basically derived from what I've practically experienced in how we use these types of libraries as build artifacts :

We work almost exclusively with PPLs for re-use libraries. In extended structures, it is often very helpful to have a level of uniformity in related library names. PPLs tend to end up in one giant global folder for ease of use/access, so it's very useful to be able to quickly find and interpret what you're seeing from that perspective.

As a consequence, we've found that prefixing related libaries with the same name has a lot of value... so something like:

  • httpClient
  • httpClient-NIAdvanced
  • httpClient-NET

Would be my strong preference.

edit: one slight alternative I've used here in the past is to "shorten" the prefix on any follow-on classes, so the logical grouping is still clear, but name length is reduced (httpClient, http-NIAdvanced, http-NET for example). This maintains the logical grouping, while not keeping things as strongly linked as @jimkring mentioned above

I've found that I've developed a fairly strong aversion to "." characters in file names (especially stuff that end up as build artifacts... like PPLs) as it can also occasionally upset filename parsers, and have shifted to either hyphens or underscores.

My other recommendation is that Library name == Primary Class name for extended classes:
httpClient-NIAdvanced.lvclass:httpClient-NIAdvanced.lvcass

This helps if there's a need to dynamically load the classes, as you only need to keep track of one thing. This would not apply to the main "interface" library which also tends to contain a couple of sub-classes, and will already be in memory anyway.

I don't have a particularly strong preference on camalCase vs PascalCase vs -/_ . I mostly use PascalCase on my day-to-day development, with "User Facing" APIs having spaces in the names (exactly for the previous point on it being easier to read/interpret).

@jimkring
Copy link
Contributor Author

jimkring commented Feb 1, 2024

@jyoung8711

My other recommendation is that Library name == Primary Class name for extended classes:
httpClient-NIAdvanced.lvclass:httpClient-NIAdvanced.lvcass

I think you may have meant httpClient-NIAdvanced.lvlib:httpClient-NIAdvanced.lvcass). Can you confirm?

Wrapping extension classes (plug-ins) in an .lvlib lends itself to building to PPLs, for sure.

One thing I like to do for for plug-in (concrete implementation) templates put the specific name ONLY in the lvlib name, like this:

httpClient-NIAdvanced.lvlib:httpClient.lvcass
httpClient-NIAdvanced.lvlib:httpHeader.lvcass
httpClient-NIAdvanced.lvlib:httpRequest.lvcass
httpClient-NIAdvanced.lvlib:httpResponse.lvcass

Note that the lvclass'es inside the lvlib all have the same name as the base interface that they are implementing.

The main reason I like this approach for plug-ins is that instantiating a new plugin from the template involves simple copying the plugin-template folder and then renaming ONLY the lvlib: httpClient-Template.lvlib -> httpClient-FancyNewClientLibrary.lvlib

This:
httpClient-Template.lvlib:httpClient.lvcass
httpClient-Template.lvlib:httpHeader.lvcass
httpClient-Template.lvlib:httpRequest.lvcass
httpClient-Template.lvlib:httpResponse.lvcass

Becomes this:

httpClient-FancyNewImplementation.lvlib:httpClient.lvcass
httpClient-FancyNewImplementation.lvlib:httpHeader.lvcass
httpClient-FancyNewImplementation.lvlib:httpRequest.lvcass
httpClient-FancyNewImplementation.lvlib:httpResponse.lvcass

I might also be OK with underscores instead of hyphens, if CamelCase is used for ClassNames:

httpClient_FancyNewImplementation.lvlib:httpClient.lvcass
httpClient_FancyNewImplementation.lvlib:httpHeader.lvcass
httpClient_FancyNewImplementation.lvlib:httpRequest.lvcass
httpClient_FancyNewImplementation.lvlib:httpResponse.lvcass

Thus, it would get built to a PPL as:

httpClient_FancyNewImplementation.lvlibp

I might also prefer to put the interface name at the end like this:

DotNet_httpClient.lvlibp
PureG_httpClient.lvlibp
NIAdvanced_httpClient.lvlibp
Basic_httpClient.lvlib

Side note: What would we do we do if a single class implements multiple interfaces?

  • Should override methods include the interface name, to avoid collisions with ancestor classes and interfaces?
  • How would we name the library? DotNet_httpClient_systemComponent.lvlibp (if it implements both the httpClient and systemComponent interfaces)

For this last one, I guess that's why I don't have a convention for including the interface name in the concrete implementation, except informally. Thus:

DotNetHttpClient.lvlibp
PureGHttpClient.lvlibp
NIAdvancedHttpClient.lvlibp
BasicHttpClient.lvlib

Then, if we want to know what interfaces a plugin supports, we would interrogate it after we load i (test which interfaces it supports by either trying to "type cast" it to known plugin types or by calling a general plugin interface with interface descriptor methods).

@jyoung8711
Copy link
Collaborator

@jyoung8711

My other recommendation is that Library name == Primary Class name for extended classes:
httpClient-NIAdvanced.lvclass:httpClient-NIAdvanced.lvcass

I think you may have meant httpClient-NIAdvanced.lvlib:httpClient-NIAdvanced.lvcass). Can you confirm?

I did. Thanks :)

Wrapping extension classes (plug-ins) in an .lvlib lends itself to building to PPLs, for sure.

One thing I like to do for for plug-in (concrete implementation) templates put the specific name ONLY in the lvlib name, like this:

httpClient-NIAdvanced.lvlib:httpClient.lvcass httpClient-NIAdvanced.lvlib:httpHeader.lvcass httpClient-NIAdvanced.lvlib:httpRequest.lvcass httpClient-NIAdvanced.lvlib:httpResponse.lvcass

Note that the lvclass'es inside the lvlib all have the same name as the base interface that they are implementing.

The main reason I like this approach for plug-ins is that instantiating a new plugin from the template involves simple copying the plugin-template folder and then renaming ONLY the lvlib: httpClient-Template.lvlib -> httpClient-FancyNewClientLibrary.lvlib

I had not seen this before, or really considered this. I like the simplicity a lot. It would definitely make templating the interface/plugin a bit easier. It's amazing the simple options you can just completely overlook sometimes...

I might also prefer to put the interface name at the end like this:

DotNet_httpClient.lvlibp PureG_httpClient.lvlibp NIAdvanced_httpClient.lvlibp Basic_httpClient.lvlib

This is the options I would prefer to avoid most, mostly from the resulting folder PPL build organization. If you have a folder with 100 PPLs in it, finding the "HTTP Client" related ppls is a bit tedious.

Side note: What would we do we do if a single class implements multiple interfaces?

  • Should override methods include the interface name, to avoid collisions with ancestor classes and interfaces?
  • How would we name the library? DotNet_httpClient_systemComponent.lvlibp (if it implements both the httpClient and systemComponent interfaces)

Good questions, and to be completely honest, I think I've rolled out interfaces a grand total of 3 times in my own code use, and none of them have inherited from multiple interfaces, so that use case hasn't occurred to me. Most of the places where I could have utilized interfaces were developed before interfaces were available as a feature, and I didn't want to refactor.

That said, I think for this project, it would be pretty unlikely to have multiple inheritance for anything provided here in order to reduce dependencies, so I don't think Id worry about that from the class/library perspective.

I'm fairly agnostic about method names, though as long as we're avoiding any "known" namespacing conflicts, I think we're in pretty good shape. I thought the namespacing that @francois-normandin used in his initial version of this was pretty reasonable.

For this last one, I guess that's why I don't have a convention for including the interface name in the concrete implementation, except informally. Thus:

DotNetHttpClient.lvlibp PureGHttpClient.lvlibp NIAdvancedHttpClient.lvlibp BasicHttpClient.lvlib

Then, if we want to know what interfaces a plugin supports, we would interrogate it after we load i (test which interfaces it supports by either trying to "type cast" it to known plugin types or by calling a general plugin interface with interface descriptor methods).

I think this is where my "edit" is coming from. I revised my previously stronger statement about the library MUST include the parent name to "it would be nice if it looked similar" in this case, just pulling the "http" nomenclature to the beginning of the name would be useful from an organizational function... which still fits, because likely all these things are really primarily Http libraries.

@jimkring
Copy link
Contributor Author

jimkring commented Feb 1, 2024

Thanks @jyoung8711! 👍

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

3 participants