Skip to content

Fixed code smell in src/posts/topics.js: Function with many parameters (count = 5): getPostsFromSet#327

Open
pskodr wants to merge 3 commits intomainfrom
pchivatx/task-1-chatbot
Open

Fixed code smell in src/posts/topics.js: Function with many parameters (count = 5): getPostsFromSet#327
pskodr wants to merge 3 commits intomainfrom
pchivatx/task-1-chatbot

Conversation

@pskodr
Copy link
Copy Markdown

@pskodr pskodr commented Mar 12, 2026

Fix Code Smells in topics.js

Problem

Static analysis (qlty smells) flagged two code quality issues in topics.js:

  1. Too many parametersgetPostsFromSet had 5 positional parameters (set, start, stop, uid, reverse), making call sites fragile and argument order easy to mix up.
  2. Variable shadowing — Inside generatePostPaths, the inner const index shadowed the outer .map() callback parameter index, creating a subtle readability hazard and potential source of confusion.

Solution

  1. Refactored getPostsFromSet to accept a single destructured options object, reducing the parameter count from 5 to 1 and making call sites self-documenting. Added a safe default of false for the optional reverse parameter.
  2. Renamed the shadowing variable in generatePostPaths from index to postIndexSuffix to eliminate the ambiguity.

Changes Made

getPostsFromSet — parameter refactor:

// Before
Posts.getPostsFromSet = async function (set, start, stop, uid, reverse) {

// After
Posts.getPostsFromSet = async function ({ set, start, stop, uid, reverse = false }) {

generatePostPaths — variable shadowing fix:

// Before
const index = postIndex === 1 ? '' : `/${postIndex}`;
return `/topic/${slug}${index}`;

// After
const postIndexSuffix = postIndex === 1 ? '' : `/${postIndex}`;
return `/topic/${slug}${postIndexSuffix}`;

Files Changed

  • topics.js

Implementation Notes

  • No callers of getPostsFromSet exist in the codebase (confirmed via full repo search), so there are no downstream call sites to update.
  • The function body of getPostsFromSet is unchanged — destructuring extracts the same named variables, so internal behaviour is identical.
  • The reverse = false default matches the logical expectation: callers that omit reverse get forward (non-reversed) ordering, consistent with the original intent.
  • The generatePostPaths fix is purely a rename — no logic change.

Testing

  • Existing unit tests covering post path generation and set retrieval should continue to pass without modification.
  • To manually verify, run:
    npm test -- --grep "posts"
  • No new test cases are required as no logic was changed, only parameter structure and variable naming.

Copilot AI review requested due to automatic review settings March 12, 2026 14:40
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors a Posts helper in src/posts/topics.js to address static-analysis code smell warnings by reducing parameter count and removing a shadowed variable in post path generation.

Changes:

  • Refactored Posts.getPostsFromSet to take a destructured options object (and default reverse to false).
  • Renamed an inner index variable in Posts.generatePostPaths to avoid shadowing the map callback parameter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +9 to 12
Posts.getPostsFromSet = async function ({ set, start, stop, uid, reverse = false }) {
const pids = await Posts.getPidsFromSet(set, start, stop, reverse);
const posts = await Posts.getPostsByPids(pids, uid);
return await user.blocks.filter(uid, posts);
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

Changing Posts.getPostsFromSet from positional args to a destructured options object is a breaking API change for any external consumers (e.g. plugins importing src/posts). It can also fail silently if a caller still passes the old (set, start, stop, uid, reverse) signature, because destructuring will yield undefined for set/start/... and then query the wrong sorted set. Consider keeping backwards compatibility by accepting both calling conventions (detect object vs non-object), or introducing a new method name for the options-object signature and leaving the existing one intact (with a deprecation path).

Copilot uses AI. Check for mistakes.
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