0

I'm finding that the largest majority of my functions in my Angular 2 app are private. for example, look at this component (my largest component - it is a form - so it has a lot of controls which are child components)

import {
    Component,
    ViewChild,
    ElementRef,
    EventEmitter,
    Output,
    OnInit
} from '@angular/core';
import {
    FormGroup,
    FormBuilder,
    Validators,
    ControlValueAccessor,
    FormControl,
    AbstractControl,
    ReactiveFormsModule
} from '@angular/forms';
import { ResultService } from '../../../services/result.service';
import { Result, FindRequest } from '../../../models/all-models';
import { HighlightDirective } from '../../../directives/highlight.directive';
import { DistanceUnitsComponent } from './distance-units.component';
import { MultiselectComponent } from 
    '../../../shared/subcomponents/multiselect-find-category.component';
import { MultiselectFindMealTypeComponent } from
    '../../../shared/subcomponents/multiselect-find-meal-type.component';
import { NumberPickerComponent } from './number-picker.component';
import { InputComponent } from '../../../shared/subcomponents/input.component';
import { AreaComponent } from 
    '../../../shared/subcomponents/area-picker.component';



@Component({
    selector: 'find-form',
    templateUrl: 'app/find-page/subcomponents/find-page/find-form.component.html',
    styleUrls: ['app/find-page/subcomponents/find-page/find-form.component.css'],
    providers: [ResultService]
})

export class FindFormComponent implements OnInit {
    @ViewChild('multiselectFindCategory')
    private _multiselectFindCategory: MultiselectComponent;
    @ViewChild('multiselectFindMealType')
    private _multiselectFindMealType: MultiselectFindMealTypeComponent;
    @ViewChild('distanceUnits') private _distanceUnits: DistanceUnitsComponent;
    @ViewChild('numberPicker') private _numberPicker: NumberPickerComponent;
    @ViewChild('areaInput') private _areaInput: AreaComponent;
    @ViewChild('keywordsInput') private _keywordsInput: InputComponent;
    @Output() private _onResultsRecieved:
    EventEmitter<Object> = new EventEmitter<Object>();
    @Output() private _onSubmitted: EventEmitter<boolean> =
    new EventEmitter<boolean>();

    private _categoryError: string = 'hidden';
    private _mealTypeError: string = 'hidden';
    private _areaError: string = 'hidden';

    private _findForm: FormGroup;
    private _submitted: boolean = false;
    private _findRequest: FindRequest;

    private _displayMealCategories: boolean = false;
    private _mealSelected: boolean = false;
    private _place: google.maps.Place;

    constructor(private _resultService: ResultService,
                    private _formBuilder: FormBuilder) {}

    ngOnInit() {
        this._findForm = this._formBuilder.group({
            categories: [null, Validators.required],
            mealTypes: [[], this._mealTypesValidator()],
            location: null,
            distanceNumber: null,
            distanceUnit: 'kilometers',
            keywords: null,
        });
    }

    private _mealTypesValidator = () => {
        return (control: FormControl) => {
            var mealTypes = control.value;
            if (mealTypes) {
                if (mealTypes.length < 1 && this._mealSelected) {
                    return {
                        mealTypesValid: { valid: false }
                    };
                }
            }
            return null;
        };
    };

    private _setAreaErrorVisibility(): void {
        if (this._areaInput.areaInput.nativeElement.value) {
            if (!this._areaInput.address) {
                this._areaError = 'visible';
            } else {
                this._areaError = 'hidden';
            }
        } else {
            this._areaError = 'hidden';
            this._findForm.get("location").setValue(null);
        }
    }

    private _onCategoriesChanged(): void {
        this._findForm.get('categories').markAsDirty();
        this._mealSelected = this._multiselectFindCategory.mealSelected;
        this._findForm.controls['mealTypes'].updateValueAndValidity();
        if (!this._mealSelected) {
            this._mealTypeError = 'hidden';
            this._findForm.get('mealTypes').setValue([]);
        }

        if (this._multiselectFindCategory.selectedCategories.length > 0) {
            this._findForm.get('categories').setValue(
                this._multiselectFindCategory.selectedCategories
            );
        } else {
            this._findForm.get('categories').setValue(null);
        }
    }

    private _onMealTypesChanged(): void {
        this._findForm.get('mealTypes').markAsDirty();
        if (this._multiselectFindMealType.selectedCategories.length > 0) {
            this._findForm.get('mealTypes').setValue(
                this._multiselectFindMealType.selectedCategories
            );
        } else {
            this._findForm.get('mealTypes').setValue([]);
        }
    }

    private _onAreaChanged(areaEntered: any): void {
        this._setStateOfDistanceControls(areaEntered.areaEntered);
        this._areaError = "hidden";
        if (areaEntered.place) {
            this._findForm.get("location").setValue({
                lat: areaEntered.place.geometry.location.lat(),
                lng: areaEntered.place.geometry.location.lng()
            })
        }
    }

    private _onDistanceUnitsChanged(distanceUnit: string): void {
        this._findForm.get("distanceUnit").setValue(distanceUnit);
    }

    private _onDistanceNumberChanged(distanceNumber: number): void {
        this._findForm.get("distanceNumber").setValue(distanceNumber);
    }

    private _setStateOfDistanceControls(areaEntered: any): void {
        if (areaEntered) {
            this._distanceUnits.isEnabled = true;
            this._numberPicker.isEnabled = true;
        } else {
            this._distanceUnits.isEnabled = false;
            this._numberPicker.isEnabled = false;
        }
        this._distanceUnits.imagePath = this._distanceUnits.imagePath;
    }

    private _getResults(form: FormGroup): void {
        var results: Result[] = [];
        results = this._resultService.getResults();
        if (results) {
            this._onResultsRecieved.emit({
                recieved: true,
                results: results,
                location: this._findForm.get("location").value
            });
        }
    }

    private _onSubmit(model: any, isValid: boolean): void {
        this._submitted = true;
        console.log(model, isValid);
        this._setAreaErrorVisibility();
        if (isValid && this._areaError === "hidden") {
            this._onSubmitted.emit(true);
            this._getResults(this._findForm);
        }
    }
}

I'm trying to be best practice here. So I have no public anythings...This seems to me like something that comes with Angular 2, because in most of my non-angular apps, I need more public things. Is my code just super loosely coupled, since no other components need to access it? I can't figure out if it is good..

But I need to unit test it and usually you don't unit test anything that's private so I'm pretty confused. I have a lot of logic that obviously should be tested.

Am I making too many things private? Or is it good practice having them private? And should I just unit test all the private functions?

BeniaminoBaggins
  • 11,202
  • 41
  • 152
  • 287

1 Answers1

1

I would suggest you to make everything public / do not add any prefix that is being exposed to your template html or of course other components. This is also the way we do it in our company and the way it is done in the official Angular2 guides.

This way you automatically test your private functions since they should be called somewhere in your public methods.

You can also create UI-Tests with things like Protractor and Selenium to cover lots of scenarios

lastWhisper
  • 462
  • 3
  • 6
  • Thanks. After doing some research into what you are saying I found it to be true. For one thing, it's best to release production code with AoT compilation, and in AoT compilation the HTML template cannot access private variables of its associated component. references: [SO answer](http://stackoverflow.com/a/39000046/3935156), [app demonstrating](https://github.com/devoto13/angular2-private-members-example), [SO answer](http://stackoverflow.com/a/39379451/3935156), [angular 2 AoT docs](https://angular.io/docs/ts/latest/cookbook/aot-compiler.html) – BeniaminoBaggins Nov 27 '16 at 22:06