2
\$\begingroup\$

Just wanted some code review for correctness here. This is a little app for plotting newborn feeding data (important to keep track of in the first 48 hours of so) that I hope to provide to a clinic, but I wanted some extra eyes on it if possible before doing so (important to get this right!). We have the times of the feeds in military time given for today and yesterday, where the newborn was born yesterday. I show a visualization in a sense to prove the code is correct to the viewer.

today = c("0800", "0645", "0300","0200")
yesterday = c("2400", "2225", "2045")

yesterdaylabels = lapply(yesterday,function(x){paste0("Yesterday ",x)})
todaylabels = lapply(today,function(x){paste0("Today ",x)})
dayslabels = c(rev(yesterdaylabels),rev(todaylabels))

#yesterday = c("2045", "2225", "2400")
#today = c("0200", "0400", "0645","0800")

yesterday = rev(yesterday)
today = rev(today)



days = list(yesterday,today)

check.day = function(day){
  lapply(day,function(x){
    if(nchar(x)==4){
      print("ok")
    }else{
      print("One of the times doesn't have 4 digits! Stopping.")
      break
    }
    })
}

for (day in days){check.day(day)}

get.day.in.min = function(day){
get.first.two = function(x){lapply(day,function(x){substr(x,1,2)})}
get.last.two = function(x){lapply(day,function(x){substr(x,3,4)})}
day.hours = as.numeric(get.first.two(day))
day.minutes = as.numeric(get.last.two(day))
day.in.minutes = day.hours*60 + day.minutes
day.in.minutes
}

alldays = c()
for (day.ix in 1:length(days)){
  newday = get.day.in.min(days[[day.ix]])+24*60*(day.ix-1)
  alldays = c(alldays,newday)
  
}

ntimes = length(alldays)
timeix=rep(0,ntimes)

daysoffset= c(alldays[2:length(alldays)],NaN)
#today = c()
timebtwnhrs = (daysoffset-alldays)/60


range = round(c(min(timebtwnhrs,na.rm=TRUE),max(timebtwnhrs,na.rm=TRUE)),1)
plot(alldays,timeix,
     pch=20,
     #pch=3,
     #xlab=paste0("Minutes \n Hours between feeds: mean = ", round(mean(timebtwnhrs,na.rm=TRUE),1)," (min = ",range[1],", max = ",range[2],")"),
     xlab=paste0("Hours between feeds: mean (range): ", round(mean(timebtwnhrs,na.rm=TRUE),1)," (min = ",range[1],", max = ",range[2],")"),
     xaxt="n",
     yaxt='n',ylab="",frame.plot = FALSE,ylim=c(0,3))

for (i in 1:length(alldays)){
text(x=alldays[i],y=0.5, labels=dayslabels[[i]],srt=-90)
}

enter image description here

\$\endgroup\$

2 Answers 2

0
\$\begingroup\$

If by correctness you mean that it does what it says it does, then, yes, it does. I don't see any reasonable cases where this would give incorrect results

\$\endgroup\$
2
\$\begingroup\$

nit: Please use a formatter like styler when sharing code with others. Idiosyncratic spacing is just a distraction that makes code a little harder to read.

timestamp library

today = c("0800", "0645", "0300","0200")
yesterday = c("2400", "2225", "2045")
...
days = list(yesterday,today)

I find this representation a little worrying, as it is not ISO-8601. I was hoping to see the use of the POSIXct library or timestamp, something well tested along those lines.

Your current focus is on the first 48 hours. But what if next year we care about the first 72 hours? Modeling YYYY-MM-DD as part of the timestamp seems only prudent.

maintainability

get.day.in.min = function(day){
get.first.two = function(x){lapply(day,function(x){substr(x,1,2)})}
get.last.two = function(x){lapply(day,function(x){substr(x,3,4)})}
day.hours = as.numeric(get.first.two(day))

That's just frightening.

Yes, it is "simple". But you're defining your own Public API for timestamps, and accepting the documentation burden. Code like this will make it more difficult for the project to recruit maintenance engineers a few months down the road when scope of project has increased.

error reporting

      print("One of the times doesn't have 4 digits! Stopping.")

This shouldn't be your responsibility. Make it someone else's problem! Use a well-tested well-documented timestamp parsing library.

no first day

There must be some protocol associated with this code -- document it.

Sometimes we won't have first day feeding observations, such as for complex deliveries that result in a brief or prolonged stay in the NICU, with intravenous feeding. The collection protocol tells us how to interpret the data. Mention it, perhaps with an URL in a comment.

ambiguity

The phrase "in military time" doesn't seem very helpful -- at best it advises us to not write down A.M. or P.M. I was kind of hoping to see Zulu in there, as military orders are often given in terms of UTC, but no.

We apparently have timestamps that use the local zone offset, which is fine, but not made explicit. Again, the ISO standard can help, by tacking on a suffix like -0500.

Some hospitals are located near time zone borders, and mothers may move across them. Many hospitals are in jurisdictions that observe Daylight Saving Time, and some babies are born near the time of such transitions. Being explicit about the particular instant in time that we made an observation is just basic data hygiene.

time of birth

I found it odd that this time does not appear, since elapsed time from birth to first feeding seems relevant.

If the data collection protocol allows, maybe also include timestamp of some milestone event that would let us pick out "long labor" births?

The yesterday / today dichotomy seems to be about when midnight happens, rather than the moment 24-hours after birth. Babies are born at all hours of the day, and we wouldn't want morning births to be treated systematically differently from evening births. The current approach seems to right-censor feeding data from evening births. That is, the current probability of finding a tibble row with "feeding at > 46 hours" is greater for a morning birth than for an evening birth.

I guess we don't have "feeding duration"? Ok. If we wished to track time rate of calories it seems like it could be relevant. Maybe timestamp and mass of excreta will be added to this dataset in future.

aesthetics

X labels

Consider tilting the timestamp about 45° for legibility.

background shading

Babies famously don't care what time it is, and whether mom has had any sleep lately. Nonetheless, it may be helpful to show vertical bars at times like midnight, or to have a light grey background extend from local dusk until dawn.

elapsed intervals

We can slice and dice the data more than one way, trying to take advantage of what the visual cortex is good at. Tufte tells us that humans are much better at comparing interval sizes when they are aligned than when they are scattered about.

You depict a timeline which is very nice and clear; please keep it. It consumes a fair amount of horizontal space, in order to have sufficient resolution to depict closely spaced feeding events.

Consider also offering a chart where a compressed X axis continues to show time-of-day, and Y axis shows elapsed time since last feeding event (or since the birth event for initial feeding, if feasible). A lucky mother would have long vertical bars depicted against a greyed out "nighttime" background, and shorter bars indicating more frequent daytime feedings. (Though the circadian effect may only really show up if observations are made for longer than the first two days.)

This alternate visualization would let us turn a text summary, e.g. "mean 1.9 hours, range 1 .. 3.8 hours", into a graphical depiction. Whiskers on a box plot is a pretty typical way to summarize a distribution.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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