Gfrktyl

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.

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.