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

[Misc] Metaprogramming attempt #100

Closed
wants to merge 2 commits into from
Closed

Conversation

invacuo
Copy link
Collaborator

@invacuo invacuo commented Oct 17, 2021

Description

Currently, we have to keep creating new classes for each endpoint that spacex api launches

These new classes are very similar to each other

This is an attempt to avoid the work to continue to write the main classes.

The work to support new endpoints will be limited to adding a new entry to this array of strings

While this undeniably comes with the downsides of metaprogramming, I am wondering if we can alleviate some of that pain by continuing to write the individual specs and updating the readme file.

@invacuo invacuo changed the title Ck wip [Misc] Metaprogramming attempt Oct 17, 2021
@invacuo
Copy link
Collaborator Author

invacuo commented Oct 17, 2021

@rodolfobandeira When you get a chance, please look at the second commit in this PR. What are your thoughts?

@rodolfobandeira rodolfobandeira added the hacktoberfest HacktoberFest label Oct 17, 2021
autoload :Capsules, 'spacex/v4/capsules'
autoload :Company, 'spacex/v4/company'
autoload :Cores, 'spacex/v4/cores'
CLASS_NAMES = %w[Capsules Company Cores Rockets].freeze
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A pattern I've seen for situations like this is to call it KLASS or klass which kind of automatically tells us that this is a dynamic handling of real classes

@rodolfobandeira
Copy link
Owner

@rodolfobandeira When you get a chance, please look at the second commit in this PR. What are your thoughts?

@invacuo It make sense to me to start thinking about a better way to handle all this code duplication. The only concern is the flexibility per model adding custom attributes wherever is necessary but if we manage to get a good result there, I'm all for it! :)

@invacuo
Copy link
Collaborator Author

invacuo commented Oct 17, 2021

The only concern is the flexibility per model adding custom attributes wherever is necessary but if we manage to get a good result there, I'm all for it! :)

I think the easiest way to do it would be to skip adding the class names of the models that need the custom attribute to the KLASS_NAMES array and add the class manually like we currently do it.

There could be other ways like using class_eval but that will depend heavily on what kind of custom attributes we are trying to add.

### Why?

Currently, we have to keep creating new classes for each endpoint that spacex api launches

These new classes are ***very** similar to each other

This is an attempt to avoid writing the main classes.

While this undeniably comes with the downsides of metaprogramming, I am wondering if we can alleviate some of that pain by continuing to write the individual specs and updating the readme file.
### Description

- Use the metaprogramming pattern to simple legacy objects using v3 endpoints

### Note
- Objects that have custom properties defined are left untouched
@invacuo
Copy link
Collaborator Author

invacuo commented Oct 18, 2021

@rodolfobandeira Pushed a second commit that applies metaprogramming to legacy objects without custom properties defined. Curious to hear your thoughts.

@invacuo invacuo closed this Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants