Title: Confusing Variable class names used for different purposes · Issue #363 · canopen-python/canopen · GitHub
Open Graph Title: Confusing Variable class names used for different purposes · Issue #363 · canopen-python/canopen
X Title: Confusing Variable class names used for different purposes · Issue #363 · canopen-python/canopen
Description: After spending quite many hours in the codebase, there is one thing I find really confusing and time consuming: Some classes are named the same thing, but with different functions. Applies to Variable and partly to Array and Record. vari...
Open Graph Description: After spending quite many hours in the codebase, there is one thing I find really confusing and time consuming: Some classes are named the same thing, but with different functions. Applies to Varia...
X Description: After spending quite many hours in the codebase, there is one thing I find really confusing and time consuming: Some classes are named the same thing, but with different functions. Applies to Varia...
Opengraph URL: https://github.com/canopen-python/canopen/issues/363
X: @github
Domain: github.com
{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Confusing Variable class names used for different purposes","articleBody":"After spending quite many hours in the codebase, there is one thing I find really confusing and time consuming: Some classes are named the same thing, but with different functions. Applies to `Variable` and partly to `Array` and `Record`. \r\n\r\n```py\r\n variable.Variable\r\n pdo.base.Variable # Inherited from variable.Variable\r\n sdo.base.Variable # Inherited from variable.Variable\r\n objectdictionary.Variable\r\n```\r\n\r\nI understand the functions for all of these. and I get it if the intention was to make a duck-type like class, but they're really not all compatible. It does not help that type checkers and linters often displays the base name of the class, not the full path. If the type checker report an error where it expects `Variable`, a small investigation is required to figure out which of these classes is the one that should be used.\r\n\r\nI would like to suggest to change the class names into unique names as follows:\r\n\r\n```py\r\n class Variable: # in variable.py\r\n class PdoVariable(Variable):\r\n class SdoVariable(Variable):\r\n class ODVariable:\r\n```\r\nLikewise for `Array` into `class SdoArray` and `class ODArray`, and `Record` into `class SdoRecord` and `class ODRecord`. I don't intend any changes to the methods and the class inheritance. If we need to keep the old names intact for compatibility, we can do so as aliases in the respective files. I can spin up a PR if there's interest for this.","author":{"url":"https://github.com/sveinse","@type":"Person","name":"sveinse"},"datePublished":"2023-04-06T22:55:10.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":1},"url":"https://github.com/363/canopen/issues/363"}
| route-pattern | /_view_fragments/issues/show/:user_id/:repository/:id/issue_layout(.:format) |
| route-controller | voltron_issues_fragments |
| route-action | issue_layout |
| fetch-nonce | v2:4779c1f3-a9e6-3e39-89b6-7a405768cf72 |
| current-catalog-service-hash | 81bb79d38c15960b92d99bca9288a9108c7a47b18f2423d0f6438c5b7bcd2114 |
| request-id | DE6E:1775E7:6A8D4CB:90B338C:6A5E2796 |
| html-safe-nonce | 857879ff8061ade50fe9cadd8df60be67c41edd3895e9b4957bd68820827aa65 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJERTZFOjE3NzVFNzo2QThENENCOjkwQjMzOEM6NkE1RTI3OTYiLCJ2aXNpdG9yX2lkIjoiNDQxNDQzNjg1NDI5NTUzMDQ2IiwicmVnaW9uX2VkZ2UiOiJpYWQiLCJyZWdpb25fcmVuZGVyIjoiaWFkIn0= |
| visitor-hmac | 3a0db8a008cd1baed96121c321867ec887c5cb85d1855daced63b10d8eb0a9df |
| hovercard-subject-tag | issue:1658142016 |
| github-keyboard-shortcuts | repository,issues,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/_view_fragments/issues/show/canopen-python/canopen/363/issue_layout |
| twitter:image | https://opengraph.githubassets.com/a6774f5eebec164aefe1133e3d9fc01e8dce596bdd1f1ffc8fc1e12fe546bfc2/canopen-python/canopen/issues/363 |
| twitter:card | summary_large_image |
| og:image | https://opengraph.githubassets.com/a6774f5eebec164aefe1133e3d9fc01e8dce596bdd1f1ffc8fc1e12fe546bfc2/canopen-python/canopen/issues/363 |
| og:image:alt | After spending quite many hours in the codebase, there is one thing I find really confusing and time consuming: Some classes are named the same thing, but with different functions. Applies to Varia... |
| og:image:width | 1200 |
| og:image:height | 600 |
| og:site_name | GitHub |
| og:type | object |
| og:author:username | sveinse |
| hostname | github.com |
| expected-hostname | github.com |
| None | e5010f4d2748a3cbef86e1580413ff14701bd99f255268dfb4a2857c77e2cc7c |
| turbo-cache-control | no-preview |
| go-import | github.com/canopen-python/canopen git https://github.com/canopen-python/canopen.git |
| octolytics-dimension-user_id | 200581454 |
| octolytics-dimension-user_login | canopen-python |
| octolytics-dimension-repository_id | 68737600 |
| octolytics-dimension-repository_nwo | canopen-python/canopen |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 68737600 |
| octolytics-dimension-repository_network_root_nwo | canopen-python/canopen |
| turbo-body-classes | logged-out env-production page-responsive |
| disable-turbo | false |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | 7d23604c4a8ce0274b4c71bb2adbe6e1c9d5904f |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width