Jl form functionality cleanup

[jleonardw9]

No description provided.

[kayla-glick]

There should be a placeholder for AM/PM in DateTime.strptime. %p should do it

[jleonardw9]

I kind of like my modifier code, so I think I'm going to leave it in.

[kayla-glick]

Can you explain what the modifier is doing? I'm a bit confused about the '0' || '10' || '11' part

[jleonardw9]

It's converting military time to regular time. So if a time string has either '0', '10', or '11' as the leading characters then it is in the morning. All else are PM.

[kayla-glick]

You could use DateTime.strptime(time, '%H:%M').strftime('%l:%M %p') to do the same thing all in 1 line

[jleonardw9]

Totally missed this comment last week. Your suggestion worked great, should be up there now.

[bmic-development]

Time format is something like 02:01:01 or 11:21:23 or 23:13:13. So it’s looking at if it start with 0 or first 2 numbers are 10,11 then it’s AM otherwise it’s PM. From: kyle-glick <notifications@github.com> Reply-To: bmic-development/sparc-request <reply@reply.github.com> Date: Monday, March 20, 2017 at 11:18 AM To: bmic-development/sparc-request <sparc-request@noreply.github.com> Cc: Subscribed <subscribed@noreply.github.com> Subject: Re: [bmic-development/sparc-request] Jl form functionality cleanup (#157) @kyle-glick commented on this pull request.

________________________________ In app/helpers/additional_details/application_helper.rb<#157 (comment)>:

@@ -1,2 +1,9 @@

module AdditionalDetails::ApplicationHelper + + def convert_time(time) + modifier = '' + ((time.first == '0') || (time[0..1] == '10') || (time[0..1] == '11')) ? modifier = ' AM' : modifier = ' PM' + + DateTime.strptime(time.gsub(':', ''), '%H%M').strftime('%l:%M') + modifier Can you explain what the modifier is doing? I'm a bit confused about the '0' || '10' || '11' part — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#157 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ACeOUbiGAfq8fDkH1O4uomHExDXTXtrYks5rnpjMgaJpZM4Mgxrm>.

…

------------------------------------------------------------------------- This message was secured via TLS by MUSC.

[kayla-glick]

You could use DateTime.strptime(time, '%H:%M').strftime('%l:%M %p') to do the same thing all in 1 line

[amcates]

@jleonardw9 I have to agree with Kyle. Using the strptime method instead of custom code would be better in this case.

[jleonardw9]

Yeah, I agree as well. Just changed it.

[kayla-glick]

I tested qr.content.delete('[]').delete('\"') and qr.content.gsub(/["\[\]]/, '') and they appear to be doing the same thing: Replacing/removing the characters []". We should combine this block and the if block into a single statement.
if ['checkbox', 'multiple_dropdown'].include?(item.item_type)
qr.content.delete('[]\"') or qr.content.gsub(/["\[\]]/, '')

[amcates]

I would label that a refactor and make another pull request. Not necessary to proceed. In the same thought you could change 'qr.content.gsub' to another method call similar to convert_time.

[kayla-glick]

Fair enough.

Code can be refactored after the fact. Works fine the way it is currently written.