Angular component for a system to store student test scores

Question

I am designing a system and in it there is a view where teachers can see a summary of test data on students. Here is the component of it:

import { Component, OnInit } from '@angular/core';
import { Router, ActivatedRoute, Params } from '@angular/router';

import { Student } from '../../models/student'
import { StudentsService } from '../../services/students.service';
import { Test } from '../../models/test'
import { TestsService } from '../../services/tests.service';

@Component({
  selector: 'app-teacher',
  providers: [ StudentsService, TestsService ],
  templateUrl: './teacher.component.html',
  styleUrls: ['./teacher.component.css']
})
export class TeacherComponent implements OnInit {

  // Local properties
  students: Student[];
  student_search_filter: string = '';
  num_items: number = 10000;
  page: number = 1;
  orderBy: string = 'last_name';
  subject: string = 'math';
  options: Object = { title : { text : 'Loading...'} };
  tests: Test[];
  testsInSession: Test[][];
  testsCount: number[] = [0,0,0];
  mean: number[] = [0,0,0];
  median: number[] = [0,0,0];
  sessions: string[] = ['Winter 2017','Fall 2016','Spring 2016'];
  scores: number[][];
  chartData: Object[][];

  constructor(private router: Router, private route: ActivatedRoute, private testService: TestsService, private studentService: StudentsService) { }

  ngOnInit() {
    this.route.params.subscribe(params => { 
        this.subject = (params['subject'].charAt(0).toUpperCase() + params['subject'].slice(1)); // Uppercase first letter and set the subject from the URL
        /*** Reset Variable ***/
        this.chartData = [];
        this.testsInSession = [];
        this.scores = [];
        for(var testsSessions = 0; testsSessions < this.sessions.length; testsSessions++){
            this.chartData[testsSessions] = [{}];
            this.testsInSession[testsSessions] = [];
            this.scores[testsSessions] = [];
        }
        this.testsCount = [0,0,0];
        this.mean = [0,0,0];
        this.median = [0,0,0];
        /**********************/
        this.createCharts(); // Load blank placeholder chart until data is loaded
        this.loadStudents();
    });
  }

  // Load the students from the API and then call function to load tests
  loadStudents() {
    this.studentService.getStudents(this.page, this.num_items, this.student_search_filter, this.orderBy)
    .subscribe(
        students => {
            this.students = students, //Assign returned students to local property
            this.loadStudentTests();
        },
        err => { console.log(err); });
  }

  // Load tests froms API of given students and then call function to generate the graph
  loadStudentTests() {
    let studentIDs: string = '';
    for(var studentsI = 0; studentsI < this.students.length; studentsI++){
        studentIDs += this.students[studentsI].student_id + '||';
    }
    studentIDs = studentIDs.slice(0, -2);

    this.testService.getTestScores([{ 'test_type': this.subject },{ 'student_id': studentIDs }])
    .subscribe(
       chartDataFromApi => { 
         this.tests = chartDataFromApi; // Assign returned tests to local property
         for(var testsSessions = 0; testsSessions < this.sessions.length; testsSessions++){
            for(var testsI = 0; testsI < this.tests.length; testsI++){
                if (this.tests[testsI].test_season == this.sessions[testsSessions]){ // If test sessions matches current session, add to these variables
                    this.testsInSession[testsSessions].push(this.tests[testsI]);
                    this.chartData[testsSessions].push({ "name":this.tests[testsI].student_id, "x":Number(this.testsCount[testsSessions]), "y": Number(this.tests[testsI].score) }, );
                    this.testsCount[testsSessions]++;
                }
            }
         }
         this.getStats();
       }, 
       err => { console.log(err); });
  }

  getStats(){
    for(var testsSessions = 0; testsSessions < this.sessions.length; testsSessions++){ 
        for(var testsI = 0; testsI < this.testsInSession[testsSessions].length; testsI++){
            this.mean[testsSessions]+=Number(this.testsInSession[testsSessions][testsI].score);
            this.scores[testsSessions].push(Number(this.testsInSession[testsSessions][testsI].score));
        }

        /* Calculate Median */
        this.scores[testsSessions].sort((a, b) => a - b);
        let lowMiddle = Math.floor((this.scores[testsSessions].length - 1) / 2);
        let highMiddle = Math.ceil((this.scores[testsSessions].length - 1) / 2);
        this.median[testsSessions] = (this.scores[testsSessions][lowMiddle] + this.scores[testsSessions][highMiddle]) / 2;
        /* -Median- */

        this.mean[testsSessions]=+(this.mean[testsSessions]/this.testsCount[testsSessions]).toFixed(2);
    }
    this.createCharts();
  }

  // Function to calcuate percentiles for the given session
  getPercentile(session, percentile){
    const index = (percentile/100) * this.scores[session].length;
    if (Math.floor(index) == index){
        return (this.scores[session][index-1] + this.scores[session][index]/2)
    } else {
        return this.scores[session][Math.floor(index)];
    }
  }

  createCharts(){
    this.options = {
        chart: {
            type: 'scatter',
            zoomType: 'xy'
        },
        title : { text : this.subject},
        xAxis: {
            title: {
                enabled: true,
                text: 'Students'
            },
            labels: {
                enabled: false
            },
            minorTickLength: 0,
            tickLength: 0
        },
        yAxis: {
            title: {
                text: 'Scores'
            }
        },
        plotOptions: {
            scatter: {
                marker: {
                    radius: 5,
                    states: {
                        hover: {
                            enabled: true,
                            lineColor: 'rgb(100,100,100)'
                        }
                    }
                },
                states: {
                    hover: {
                        marker: {
                            enabled: false
                        }
                    }
                },
                tooltip: {
                    headerFormat: '<b>{series.name}</b><br>',
                    pointFormat: '{point.name}<br />Score: {point.y}'
                }
            }
        },
        series: [{
            name: this.sessions[0],
            color: 'rgba(223, 83, 83, .5)',
            data: this.chartData[0]
        },{
            name: this.sessions[1],
            color: 'rgba(119, 152, 191, .5)',
            data: this.chartData[1]
        },{
            name: this.sessions[2],
            color: 'rgba(80, 180, 50, .5)',
            data: this.chartData[2]
        }],
        credits: {
            enabled: false
        }
      };
  }

}

Supporting files:

• student.ts model
• students.service.ts service
• tests.service.ts service

My specific worries are if I should be doing the average, percentile, etc calculations here or in another file. Also, is there a better way for me to use/set the local properties? I want to use them throughout this view, but it seems I need to reset them in ngOnInit for when I pick a different subject.

Answer 1

It looks like you're doing everything in just one component. That's now how you're supposed to be creating Angular Components.

There should be one rule of thumb to keep in mind while creating components. That is, "The purpose of a Component should only be to present data to a user and let the user interact with the data"

Keeping this thumb-rule in mind lets us create what are called Skinny Components.

That being said, the only code that should be present in your component should be that of getting the data from a service and then showing it to the user. If there is an operation that needs to be performed on the data, it should be delegated to a service.

Another thing that I saw in the code was, providers: [ StudentsService, TestsService ],. Not sure if you've done this intentionally. But doing this will create a separate instance of these services for TeacherComponent and its children.

One last thing that I'd point out is that there are several things in the TeacherComponent like createCharts, getPercentile, getStats etc. All these should be done as a part of a child component which will then rely on a service to get the appropriate data in the appropriate form.

Hope this helps. Please let me know if you need more comments to get this better, more loosely coupled and with better separation of concerns. It would also help if you could create a StackBliz Project and share it here so that others could look into it.