Skip to content

Log the match details at the start of the log file#66

Open
npetrangelo wants to merge 2 commits intomasterfrom
log-match-details
Open

Log the match details at the start of the log file#66
npetrangelo wants to merge 2 commits intomasterfrom
log-match-details

Conversation

@npetrangelo
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown

@AndrewItIs AndrewItIs left a comment

Choose a reason for hiding this comment

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

Looks good I think there are few small things that could be changed; however, the code could run with out the changes I requested.

String event = station.getEventName();
String type = station.getMatchType().name();
String match = Integer.toString(station.getMatchNumber());
Logger.getRootLogger().info(String.join(" ", event, type, match));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think that the delimiter should probably be something other than just a space to prevent confusion when looking through quickly, but it probably won't matter too much in this case.

DriverStation station = DriverStation.getInstance();
String event = station.getEventName();
String type = station.getMatchType().name();
String match = Integer.toString(station.getMatchNumber());
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm not sure what the log match data is for but if it's for what I think it is, I think that there should be an if statement to check if any of the strings are null, if they are they should be reassigned as "No event data found" or "No match data found" instead of catching the null pointer by the string.join. I think this will be able to eliminate the need for a try-catch and it will be able to tell what specifically went wrong, further it will tell us other match data instead of just giving a blanket "no match data".

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.

3 participants