taustation-fan / university

Machine-readable data of University courses
https://education.tauguide.de/
3 stars 8 forks source link

add course total cost info #16

Closed particle closed 6 years ago

particle commented 6 years ago

somewhat naive PR as i haven't tested. attempting to close https://github.com/taustation-fan/taustation-fan.github.io/issues/8 which i submitted to the wrong repo as i'm still figuring this site/repo structure out.

moritz commented 6 years ago

Thanks @particle, I've deployed it.

moritz commented 6 years ago

@particle there's still something a bit off here:

 You have spent at least 0140005600420016800168001120084004200168002800200840012600420084001680042008400168002800016800168001680084002800200200840028008400280028002800402240012600840042004200420028004040202004002020140005600420016800168001120084004200168002800200840012600420084001680042008400168002800016800168001680084002800200200840028008400280028002800402240012600840042004200420028004040202004002020 credits on your education. 

I don't think I've spent that much :-)

particle commented 6 years ago

LOL like i said... untested. how did that happen, i wonder?

dot-sent commented 6 years ago

Looks like string concatenation vs integer addition to me.

On Mon, 24 Sep 2018 at 17:35, Jerry Gay notifications@github.com wrote:

LOL like i said... untested. how did that happen, i wonder?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/taustation-fan/university/pull/16#issuecomment-424018913, or mute the thread https://github.com/notifications/unsubscribe-auth/AFsl5a775DLz94sYkbdK5Isqm_YkQxK9ks5uePvMgaJpZM4W1Xyw .

-- Best regards, Sergey Kudriavtsev

moritz commented 6 years ago

The weird thing is that course_credits is clearly initialized as a number, and the only operator used on it is +=.

particle commented 6 years ago

i've just added parseInt, and pushed, however i'm not sure if this PR is still valid

moritz commented 6 years ago

@particle I've cherry-picked (Jerry-picked? :D) the commit to master. Thanks :-)

Perleone commented 6 years ago

@particle there's still something a bit off here:

 You have spent at least 0140005600420016800168001120084004200168002800200840012600420084001680042008400168002800016800168001680084002800200200840028008400280028002800402240012600840042004200420028004040202004002020140005600420016800168001120084004200168002800200840012600420084001680042008400168002800016800168001680084002800200200840028008400280028002800402240012600840042004200420028004040202004002020 credits on your education. 

I don't think I've spent that much :-)

A look at the HTML source code reveals the problem: The YAML numbers are transformed into JSON strings: "cost" : "14000",

So the original fault lies within render.pl, probably either one of YAML::XS or JSON::XS.

particle commented 6 years ago

i don't have this issue, since adding parseInt(). are you sure you're running head of master?

Perleone commented 6 years ago

parseInt() is merely the fix for the underlying problem I described above. Mostly an academic discussion now. ;-)