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

Refactor getSubTags and getParentTags in order to use cheerio maps #86

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

darthnithin
Copy link
Contributor

Refactored getSubTags

@darthnithin darthnithin changed the title Refactor getSubTags in order to use cheerio maps Refactor getSubTags and getParentTags in order to use cheerio maps Jan 6, 2025
username: username,
pseud: decodeURI(pseud),
anonymous: false,
});
});
} as Author

Choose a reason for hiding this comment

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

We should figure out why you need "as Author" here. What happens if change line 42 to be

    return authorLinks.map((_, element): Author => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'm pretty sure i don't need to. I'll check tomorrow

const description = $seriesPage("dl.series blockquote.userstuff").html();
return description ? description.trim() : null;
export const getSeriesDescription = ($seriesPage: SeriesPage): string | null => {
return $seriesPage("dl.series blockquote.userstuff").html()?.trim() || null;

Choose a reason for hiding this comment

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

[...].trim() ?? null, unless you want empty string to become null.

Copy link
Contributor Author

@darthnithin darthnithin Jan 9, 2025

Choose a reason for hiding this comment

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

that's the original behavior which I preserved.

if description is an empty string the conditional is falsy

});
});
return $tagPage(".sub > ul.tags > li").map((_, element) => {
if($tagPage($tagPage(element).has("ul.tags")).length) {

Choose a reason for hiding this comment

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

Change this for an early if return (the thing we discussed on Discord) if you can :)

return pseudsArray.join(PSEUD_SUFFIX);
return pseuds.length !== 0
? pseuds.map((_, element) => decodeURI(element.attribs.href.match(/users\/(.+)\/pseuds\/(.+)/)![2]))
.get()

Choose a reason for hiding this comment

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

is there a reason why you changed this? it makes it harder to understand what's going on.

I would

  1. if pseuds is empty, immediately return ""
  2. if not, do pseuds.map like you're doing now
  3. keep the inner function (const url = element.attribs.href; etc) as is, but return instead of adding to the array.

return authors;
}
}).get()
: [] as Author[];

Choose a reason for hiding this comment

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

Ternary operators are useful, but they can be cognitively heavy when large. If you immediately return [] as soon as you know that the authorNode is empty, that will make the code much easier to reason about. Same in the code below.

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