Skip to content
This repository has been archived by the owner on Jul 10, 2024. It is now read-only.

Team 6: Reader and Lexer #5

Closed
wants to merge 0 commits into from
Closed

Conversation

Shiny1708
Copy link
Contributor

No description provided.

Copy link
Owner

@marcauberer marcauberer left a comment

Choose a reason for hiding this comment

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

Just some minor review comments. Nice work!

@Override
public Token getToken() {

if (tokenText.isEmpty() && reader.isEOF()) {
Copy link
Owner

Choose a reason for hiding this comment

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

This logic should be part of advance. This method should only return the member variable that holds the current token.

return null;
}

private void advanceStringLiteral() {
Copy link
Owner

Choose a reason for hiding this comment

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

This is done by the StringLiteralStateMachine

addState(start);
addState(end);

Range digits = new Range('0', '9');
Copy link
Owner

Choose a reason for hiding this comment

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

Please avoid accepting leading zeroes

public void init() {
State stateStart = new State("Start");
stateStart.setStartState(true);
stateStart.setAcceptState(true);
Copy link
Owner

Choose a reason for hiding this comment

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

This looks suspicious. The accepting state shouldn't be the start state here.

Reader reader = new Reader(new File("src/test/resources/test.txt"));
Lexer lexer = new Lexer(reader);

lexer.advance();
Copy link
Owner

Choose a reason for hiding this comment

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

Advance should be done automatically by expect, so you don't need to call both

@Test
public void testGetChar() throws FileNotFoundException {
Reader reader = new Reader(testFile);
reader.getChar();
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe you should assert somehow if the expectations afterwards are met

@marcauberer
Copy link
Owner

Tests should work though!

@marcauberer marcauberer changed the title Jeremias Nils Nick Simon Team 6: Reader and Lexer Apr 18, 2024
@Shiny1708 Shiny1708 closed this Apr 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants