Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

rogerKaelin
Copy link

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

…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
Copy link
Owner

@eddex eddex left a 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!

Comment on lines +18 to +20
isNotInfoSemester: (hsluModuleName) => {
return !hsluModuleName.includes('INFO')
},
Copy link
Owner

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.

Suggested change
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);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return hsluModuleName.split('.')[2].includes('F') && ModuleParser.isNotInfoSemester(hsluModuleName);
return hsluModuleName.split('.')[2].includes('F') && !ModuleParser.isInfoModule(hsluModuleName);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants