-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
firstModule detection fix #101
base: master
Are you sure you want to change the base?
Conversation
…nt-1283029378 -Fixed the first semester calculation -> Check comment for detailed description of fix -Also general cleanup as my ide warned me about simple things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution. Both me and @Lextum are no longer studying at HSLU, therefore we can't test the changes and need to rely on you to test this properly!
isNotInfoSemester: (hsluModuleName) => { | ||
return !hsluModuleName.includes('INFO') | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a method returns a bool
, always think of what it should check, not what it shouldn't. This makes the code more readable.
isNotInfoSemester: (hsluModuleName) => { | |
return !hsluModuleName.includes('INFO') | |
}, | |
isInfoModule: (hsluModuleName) => { | |
return hsluModuleName.includes('INFO') | |
}, |
const includesH = hsluModuleName.split('.')[2].includes('H'); | ||
const includesF = hsluModuleName.split('.')[2].includes('F'); | ||
return includesH || includesF ? includesH : undefined; | ||
return hsluModuleName.split('.')[2].includes('H') && ModuleParser.isNotInfoSemester(hsluModuleName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return hsluModuleName.split('.')[2].includes('H') && ModuleParser.isNotInfoSemester(hsluModuleName); | |
return hsluModuleName.split('.')[2].includes('H') && !ModuleParser.isInfoModule(hsluModuleName); |
* Modules are marked with 'F' for 'Frühlingssemester' (spring). | ||
*/ | ||
isSpringSemester: (hsluModuleName) => { | ||
return hsluModuleName.split('.')[2].includes('F') && ModuleParser.isNotInfoSemester(hsluModuleName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return hsluModuleName.split('.')[2].includes('F') && ModuleParser.isNotInfoSemester(hsluModuleName); | |
return hsluModuleName.split('.')[2].includes('F') && !ModuleParser.isInfoModule(hsluModuleName); |
Fixed the firstModule detection to also detect spring starts and ignore any module containing "INFO" in the "anlassnumber" field. For more details on the fix check out my comment: #99 (comment)
Related issues are:
#99
#95