sfbrigade / compass

Setting and tracking goals for students with disabilities
https://staging.compassiep.com/
14 stars 14 forks source link

Argument renaming #456

Open katconnors opened 3 hours ago

katconnors commented 3 hours ago

Argument renaming in backend/routers for clarity (note that the original comment references case_manager, but naming is also in the para file as well)- see comment below from PR #433:

"This is a lot of arguments, whose values don't look like the names of the parameters. Can we consider changing this function to take keyword arguments, so that it's clear for example that req.ctx.auth.session.user?.name ?? "" is being fed as case_manager_name?"

_Originally posted by @canjalal in https://github.com/sfbrigade/compass/pull/433#discussion_r1807002240_

canjalal commented 3 hours ago

Where I was working last time, once we had over 3 arguments, we used keyword arguments, like this:

const newCourse = await CoursesRepopsitory.create({
  gradeIds: values.grades,
  name: values.courseName,
  schoolId: values.teacherMemeberships?.value,
  subjectIds: values.usbbjects,
});

where CoursesRepository.create is defined as:

interface NewCourseData {
  gradeIds: number[];
  name: string;
  schoolId: number;
  subjectids:number[];
}

class CoursesRepository {
  static async create({
    gradeIds,
    name,
    schoolId,
    subjectIds,
  }: NewCourseData): Promise<CourseWithGradeAndSubjectData | void> {
...