SonarLint says I should remove or rename the parameter being used-CodePudding

Sonar lint says:

Remove the unused function parameter "customParams" or rename it to "_customParams" to make intention explicit.

But, the "customParams" parameter is being used in the "getNextUrl" method, what are I doing wrong?

class Pagination {
  private readonly maxPage: number

  constructor (
    private readonly data: Array<{}>,
    private readonly currentPage: number,
    private readonly limit: number,
    private readonly count: number,
    private readonly urlRoute: string,
    private readonly customParams: {} = {}
  ) {
    this.maxPage = Math.ceil(count / limit)
  }

  private getNextUrl (): string | null {
    if (this.currentPage >= this.maxPage) {
      return null
    }
    const query = toQueryString({
      page: this.currentPage   1,
      limit: this.limit,
      ...this.customParams
    })
    return getUrl(this.urlRoute)   '?'   query
  }

CodePudding user response：

I think it means that you define it in the constructor but never pass something to that Pagination() constructor for customParams so you allow for customParams to be sent in like:

const paginator = new Pagination(
    [{}], //    private readonly data: Array<{}>,
    0, // private readonly currentPage: number,
    100, // private readonly limit: number,
    87, // private readonly count: number,
    '/pages', // private readonly urlRoute: string,
    { someCustom: 3000 }, // private readonly customParams: {} = {}
  );

It is possible to fill out customParams but nowhere in your code did you actually provide any so... customParams is only ever defined as {} which is how you initiated it. If there's nobody using customParams yet, why bother defining it? (YAGNI = you aren't going to need it).

Perhaps actually use customParams or set it at the top of the file like you did with maxPage or don't include it at all.

If you were to run cmd shift f (in Mac VS-Code/Sublime/Atom/etc your code editor) you can search for all usages of the Pagination class. It seems that there is no use case currently (neither dynamic nor hardcoded that actually defines customParams aka the 6th argument passed in the constructor.

Also, while we're discussing code-styling some linters will fault you for not using the concept of named params a single object parameter which accepts all the potentially optional params (where you might have a long list of 4 params) because that strategy it is easier to remember the order of the params. It will be hard for the user of the software to remember that they can pass a 6th param for customParams (especially if it is optional). So just as a potential improvement try defining functions with many params (usually more than 3) with just 1 param that is an object and can take optional params.


interface PaginationOptions {
 data: Array<{}>,
 currentPage: number,
 limit: number,
 count: number,
 urlRoute: string,
 customParams?: Record<string, any>
}

constructor({ data, currentPage, limit, count, urlRoute, customParams = {} }: PaginationOptions)