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

Updated StateOfTexas #14

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Updated StateOfTexas #14

wants to merge 1 commit into from

Conversation

skebila
Copy link

@skebila skebila commented Feb 9, 2023

I used a for loop to loop through the collection_ids obtained from the API call to the collections_catalog endpoint. For each id in collection_ids, I made two API calls, one to collections/{id} and one to resources?collection_id={id}. I then destructured the relevant information from the responses of both API calls, such as publication_date, description, and name.

Copy link
Contributor

@Nefsen402 Nefsen402 left a comment

Choose a reason for hiding this comment

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

Nice work! I like the methodical programming in splitting up the variables in extremely small pieces. However, it's worth noting that it can be overkill.


// Loop through each result
results.map(item => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Although a problem with the original code, The usage of map is inappropriate here. Please use forEach instead.

});
});
}
// Return the data array
Copy link
Contributor

Choose a reason for hiding this comment

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

These comments are unnecessary, they don't add any more insight to what is happening.

// Convert the response to a JSON object
const catalogResult = await collectionsCatalogRequest.json();
// Extract the collection ids from the result
let collection_ids = catalogResult['results'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Please prefer to use obj.results over obj['results'].

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

Successfully merging this pull request may close these issues.

2 participants