1
\$\begingroup\$

I am refactoring a very old web page which calculates bolus and infusion drug doses for children from premature through to young adulthood.

One of the most common drug errors is accidental 10 fold under or overdosing. Given the form needs both weight and age to produce an appropriate chart, it seemed wise to look up statistical data and work out the centile (cumulative standard normal distribution) of weight for age and apply soft and hard limits, with a checkbox saying that values exceeding the soft limits have been double checked. The full code (including the helper functions referenced below) is on GitHub.

The questions are:

  • Is the extensive use of wrapping data p_varName & computed getters & setters boilerplate for each 'watched' variable the best approach, so that any time age, gender, weight or gestation change, the centile (+/- warnings as required) are updated?
  • Is it the correct thing to have the centile range [bootstrap] alert as a component? This will not be reused, but it felt cleaner to compartmentalise the code/objectives. Is there a better way to achieve this separation?
  • Is emitting the 'validCentile' event when validity changes the best way - or should I use a slot with a 'valid' property on the parent scope (weightage)?
  • All the data properties are 'flat' - i.e. nothing nested - would readability/debugability/general finesse be improved by nesting properties?

Any thoughts (including those not related to the points above) very much appreciated.

<!-- src/components/weightage.vue -->
<template>
    <div class="weightAge">
        <fieldset class="form-group">
            <div class="form-row">
                <legend class="col-form-label col-sm-2 pt-0">Gender</legend>
                <div class="col-sm-10 gender">
                    <div class="form-check form-check-inline" id="male">
                        <input type="radio" name="gender" id="maleRadio" :value="true" class="form-check-input" v-model="isMale" />
                        <label class="form-check-label" for="maleRadio">
                            Male
                        </label>
                    </div>
                    <div class="form-check form-check-inline" id="female">
                        <input type="radio" name="gender" id="femaleRadio" :value="false" class="form-check-input" v-model="isMale" />
                        <label class="form-check-label" for="femaleRadio">
                            Female
                        </label>
                    </div>
                </div>
            </div>
        </fieldset>
        <div class="form-group form-row">
            <label class="col-sm-2 col-form-label" for="Weight" >Weight</label>
            <div class="input-group col-sm-10">
                <input id="Weight" type=number min="0.2" max="400" class="form-control" v-model.number="weight" required />
                <div class="input-group-append">
                    <div class="input-group-text">Kg</div>
                </div>
            </div>
        </div>
        <centile-range :lowerCentile="lowerCentile" :upperCentile="upperCentile"></centile-range>
        <div class="form-group form-row">
            <label class="col-sm-2 col-form-label" for="dob">Date of Birth</label>
            <div class="col-sm-10">
                <input class="form-control" type="date" :max="today" v-model="dob" id="dob" />
            </div>
            <span class="text-danger"></span>
        </div>
        <fieldset class="form-group">
            <div class="form-row">
                <legend class="col-form-label col-sm-2 pt-0">Age</legend>
                <div class="col-sm-10 age form-inline">
                        <div class="input-group mb-1">
                            <input type="number" step="1" min="0" max="130" v-model.number="years" id="age-years" class="form-control" />
                            <div class="input-group-append">
                                <div class="input-group-text">years</div>
                            </div>
                        </div>
                        <div class="input-group mb-1">
                            <input type="number" step="1" min="0" max="37" v-model.number="months" id="age-months" class="form-control" />
                            <div class="input-group-append">
                                <div class="input-group-text">months</div>
                            </div>
                        </div>
                        <div class="input-group mb-1">    
                            <input type="number" step="1" min="0" max="90" v-model.number="days" id="age-days" class="form-control" />
                            <div class="input-group-append">
                                <div class="input-group-text">days</div>
                            </div>
                        </div>
                </div>
            </div>
        </fieldset>
        <div class="form-group form-row">
            <label class="col-sm-2 col-form-label" for="GestationAtBirth" >Birth Gestation</label>
            <div class="input-group col-sm-10">
                <input id="GestationAtBirth" type=number min="23" max="43" step="1" class="form-control" v-model="gestation" required/>
                <div class="input-group-append">
                    <div class="input-group-text">weeks</div>
                </div>
            </div>
            <small id="nhiHelp" class="form-text text-muted">for checking weight is correct for age</small>
        </div>
    </div>
</template>

<script lang="ts">
import Vue from 'vue'
import * as moment from 'moment'
import * as ageHelper from './AgeHelper'
import { UKWeightData } from '../../CentileData/UkWeightData'
import './centilerange.vue'

const _wtCentiles = new UKWeightData(); 
export default Vue.extend({
    data:function(){
        return {
            p_weight: null as null | number,
            p_isMale: null as null | boolean,
            p_gestation: 40,
            today: moment().format(ageHelper.dateFormat),
            p_dob: '',
            p_years: null as null | number,
            p_months: null as null | number,
            p_days: null as null | number,
            ageDaysLb:null as null| number,
            ageDaysUb:null as null | number,
            lowerCentile:null as null | number,
            upperCentile: null as null | number
        }
    }, 
    //components:{centilerange},
    computed:{
        'weight': {
            get: function (this:any) {
                return this.p_weight;
            },
            set: function (newVal: any) {
                this.p_weight = newVal || newVal === 0
                    ?newVal as number
                    :null;
                this.setCentiles();
            }
        },
        'gestation': {
            get: function (this:any) {
                return this.p_gestation;
            },
            set: function (newVal: number) {
                this.p_gestation = newVal;
                this.setCentiles();
            }
        },
        'isMale': {
            get: function (this:any) {
                return this.p_isMale;
            },
            set: function (newVal: any) {
                this.p_isMale = typeof newVal  === 'boolean'
                    ?newVal
                    :null;
                this.setCentiles();
            }
        },
        'days': {
            get: function (this:any) {
                return this.p_days;
            },
            set: function (newVal: any) {
                this.p_days = newVal || newVal === 0
                    ?newVal as number
                    :null;
                this.setAgeBounds();
            }
        },
        'months': {
            get: function (this:any) {
                return this.p_months;
            },
            set: function (newVal: number | string) {
                this.p_months = newVal || newVal === 0
                    ?newVal as number
                    :null;
                this.setAgeBounds();
            }
        },
        'years': {
            get: function (this:any) {
                return this.p_years;
            },
            set: function (newVal: number | string) {
                this.p_years = newVal || newVal === 0
                    ?newVal as number
                    :null;
                this.setAgeBounds();
            }
        },
        'dob': {
            get: function (this:any) {
                return this.p_dob;
            },
            set: function (newVal: string) {
                this.p_dob = newVal;
                const ageData = ageHelper.daysOfAgeFromDob(newVal);
                if (ageData){
                    this.p_years = ageData.years;
                    this.p_months = ageData.months;
                    this.p_days = ageData.days;
                    this.ageDaysUb = this.ageDaysLb = ageData.totalDays;
                }
                this.setCentiles();
            }
        }
    },
    methods:{
        setAgeBounds(){
            let bounds = ageHelper.totalDaysOfAge(this.p_years, this.p_months, this.p_days);
            if (bounds === null){
                this.ageDaysLb = this.ageDaysUb = null;
            } else {
                this.ageDaysLb = bounds.Min;
                this.ageDaysUb = bounds.Max;
            }
            this.setCentiles();
        },
        setCentiles(){
            if (!this.p_weight || this.ageDaysLb===null){
                this.lowerCentile = this.upperCentile = null;
            } else {
                this.lowerCentile = 100 * _wtCentiles.cumSnormForAge(this.p_weight, this.ageDaysUb as number, this.p_isMale === false ? false : true, this.p_gestation);
                this.upperCentile = this.ageDaysUb === this.ageDaysLb && this.p_isMale !== null
                    ? this.lowerCentile
                    : 100 * _wtCentiles.cumSnormForAge(this.p_weight, this.ageDaysLb, !!this.p_isMale, this.p_gestation);
            }
        }
    },
    created: function () {
        let self = this;
        ageHelper.onNew('day', function (newDate) {
            self.today = newDate;
        })
    }
});
</script>

And the child component which displays the centiles, and if necessary the checkbox to acknowledge any warnings:

<!-- src/components/centilerange.vue -->
<template>
    <div class="centile" v-show="lowerVal" :class="{alert: true, 'alert-info':!warnCrossed, 'alert-warning':warnCrossed && !limitCrossed, 'alert-danger':limitCrossed }">
        <span class="lower">{{lowerVal}}<sup>{{lowerSuffix}}</sup></span>
        <span v-if="upperVal!==lowerVal">
            - 
            <span class="upper">{{upperVal}}<sup>{{upperSuffix}}</sup></span>
        </span>
        <span class="centileDescr">
            centile
        </span>
        <div v-if="warnCrossed">
            only 1 in {{denominator}}
            <span v-if="largeNumWord">
                {{largeNumWord}} 
                <small>
                    (10<sup>{{largeNumExp10}}</sup>) 
                </small>
            </span>
            weigh {{moreLess}}. 
            <div v-if="!limitCrossed" class="form-check form-check-inline">
                <input type="checkbox" id="acceptCentile" :checked="acceptWarning" class="form-check-input" required/> 
                <label for="acceptCentile" class="form-check-label">I confirm this is the correct weight</label>
            </div>
        </div>
    </div>
</template>


<script lang="ts">
import Vue from 'vue'
import { largeNumberWords, getSuffix } from '../../Utilities/NumberToWords';

const warnCentileUbound = 99;
const warnCentileLbound = 1;
const limitCentileUbound = 100 - 1e-7;
const limitCentileLbound = 1e-12;
export default Vue.component("centile-range",{
    props:[
        'lowerCentile',
        'upperCentile'
    ],
    data:function(){
        return {
            p_warnCrossed: false,
            p_limitCrossed: false,
            p_isValid:false,
            p_acceptWarning:false,
            lowerVal: '',
            lowerSuffix: '',
            upperVal:'',
            upperSuffix:'',
            moreLess:'',
            denominator:'',
            largeNumWord:'',
            largeNumExp10:null as null | number
        }
    },
    computed:{
        limitCrossed:{
            get:function(this:any){
                return this.p_limitCrossed;
            },
            set:function(newVal:boolean){
                this.p_limitCrossed = newVal;
                this.setValidity();
            }
        },
        warnCrossed:{
            get:function(this:any){
                return this.p_warnCrossed;
            },
            set:function(newVal:boolean){
                this.p_warnCrossed = newVal;
                this.setValidity();
            }
        },
        acceptWarning:{
            get:function(this:any){
                return this.p_acceptWarning;
            },
            set:function(newVal:boolean){
                this.p_acceptWarning = newVal;
                this.setValidity();
            }
        }
    },
    watch:{
        lowerCentile:function(newVal:number | null){
            this.setWarnings();
            if (newVal || newVal === 0){
                let c = centileText(newVal);
                this.lowerVal = c.centile;
                this.lowerSuffix = c.suffix;
            } else {
                this.lowerVal = this.lowerSuffix = '';
            }
            //this.sameVal = this.lowerVal === this.upperVal;
        },
        upperCentile:function(newVal: number | null){
            this.setWarnings();
            if (newVal || newVal === 0){
                let c = centileText(newVal);
                this.upperVal = c.centile;
                this.upperSuffix = c.suffix;
            } else {
                this.upperVal = this.upperSuffix = '';
            }
            //this.sameVal = this.lowerVal === this.upperVal;
        }
    },
    methods:{
        setValidity(){
            const isValid = !(this.p_limitCrossed || (this.p_warnCrossed && !this.p_acceptWarning));
            const emit = this.p_isValid !== isValid;
            this.p_isValid = isValid;
            if (emit){
                this.$emit("validCentile", this.p_isValid);
            }
        },
        setWarnings(){
            const self = this;
            if (this.upperCentile === null && this.upperCentile === null){
                this.warnCrossed = this.limitCrossed = false;
                clearNum();
            } else {
                let minVal = this.lowerCentile === null
                    ? this.upperCentile
                    : this.lowerCentile;
                 let maxVal = this.upperCentile === null
                    ? this.lowerCentile as number
                    : this.upperCentile;
                this.limitCrossed = maxVal < limitCentileLbound || minVal >limitCentileUbound;
                this.warnCrossed = maxVal < warnCentileLbound || minVal > warnCentileUbound;
                if (this.limitCrossed || this.warnCrossed){
                    let denom;
                    if (maxVal < warnCentileLbound){
                        denom = 100/maxVal;
                        this.moreLess = "less";
                    } else {
                        denom = 100/(100-maxVal);
                        this.moreLess = "more";
                    }
                    let words = largeNumberWords(denom);
                    this.denominator = words.digits;
                    this.largeNumWord = words.suffixName;
                    this.largeNumExp10 = words.exp10;
                } else {
                    clearNum();
                }
            }

            function clearNum(){
                self.moreLess= self.denominator=self.largeNumWord='';
                self.largeNumExp10 = null;
            }
        }
    }
});

function centileText(centile:number){
    let l = Math.round(centile);
    if (l < 1){
        return {centile:"<1", suffix: "st" }
    }
    if (l >= 100){
        return {centile:">99", suffix: "th" }
    }
    return {centile:l.toString(), suffix: getSuffix(l)};
}
</script>
\$\endgroup\$
1
\$\begingroup\$

I will start off that I can read typescript to a certain degree, but I have never used typescript with Vue before, so I can't really comment on your use of typescript here.

Side-effects in computed properties

All your computed properties have side-effects in their set-function. This side-effect seems to result in calculating the value of two different variables. Try to avoid this: Keep your computed properties simple and only calculate that value in that computation function. If another variable depends on your computed property, just put that one in a computed property as well. Vue will handle the dependencies and recalculate it.

If you need to have some kind of side-effect, for example to invoke a debounced calculation for something that takes a significant amount of time or to do an api call, create a watcher instead:

import myApiCall from 'api/myApi';

// ES5 syntax
export default {
  computed: {
    somethingImportant() {
      return Math.floor(Math.random() * 10);
    }
  },

  watch: {
    somethingImportant(newValue) {
      myApiCall(newValue).then((result) => {
        console.log(result);
      });
    }
  }
}

By doing it this way, you make the code easier to read. The computed property now just calculates something from other data. The watcher now takes care of all the side-effects.

Your questions

Is the extensive use of wrapping data p_varName & computed getters & setters boilerplate for each 'watched' variable the best approach, so that any time age, gender, weight or gestation change, the centile (+/- warnings as required) are updated?

See above. The better approach is to use computed properties and watchers.

Is it the correct thing to have the centile range [bootstrap] alert as a component? This will not be reused, but it felt cleaner to compartmentalise the code/objectives. Is there a better way to achieve this separation?

When you can extract something that feels "complete" out of a component and do not have to pass half of the internal state to that component as well, extracting such a part to a separate component could be a good choice. If you need to pass a significant amount of the internal state to the new component, it means that the component is not as separate as you once thought.

Extracting a component allows you to extract away its internal state away. If done correctly, it means that you now have two simpler components, which are easier to understand and reason about. I think your extraction of the centilerange component is a good choice.

Is emitting the 'validCentile' event when validity changes the best way - or should I use a slot with a 'valid' property on the parent scope (weightage)?

If you need to pass data from a child to a direct parent, use events. If you need to pass data from a component to an ancestor, or to a sibling, consider using either an event bus (e.g. a dummy component bound to this.$bus), or a vuex store mutation.

If you use the value property and the input event, you can use the v-model syntactic sugar in the parent component. If you only use the event to communicate a value back, this is probably the best way.

All the data properties are 'flat' - i.e. nothing nested - would readability/debugability/general finesse be improved by nesting properties?

This is mostly personal preference. I would recommend grouping similar properties under a single object, to avoid having to have an ever increasing list of properties that you need to pass to a child object. A perfect example might be a configuration object. Keep in mind that you need to document somehow what keys are required and what keys are optional in that object, and what they roughly mean. Since you are passing an object around, this information is not as immediately clear as passing several separate properties around.

Some other feedback

You are using the same overall order of component "parts" (data, methods, lifecycle methods), which is a good thing.

Consider factoring out the inline object for your class definition, and move it to a computed property instead. This will make it easier to add or remove classes, and make it clearer when you make dynamic class names, and make your html lines shorter. Consider moving attributes to a new line when your lines get too long.

Consider using ES6 syntax more. I see you are using let/const and destructuring, but there are much more nice things in ES6. Things like method shorthands and the spread operator come to mind.

Consider factoring out any function that is not in the component export to a separate file. If you don't believe it belongs in the internal structure of a component, it probably does not belong in the single component file.

Your second component file needs some cleaning up. I don't think it is necessary to define a function inside setWarnings. Just define it in the methods object and call it with this.clearNum(). There are some operators that have inconsistent whitespace around them.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.