-
Notifications
You must be signed in to change notification settings - Fork 35
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
sba school management system - ghassan nasr #7
base: master
Are you sure you want to change the base?
Conversation
…processing of the .sql scripts in the resources folder.
…ally with each run of the application. Just need to handle some cases such as registering for the same course twice, and displaying courses better (right now just printing the object references).
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.
Well done. The base implementation looks agreeable.
I strongly advise attempting to get some amount of testing coverage.
return connectionBuilder | ||
.setDatabaseName(getDatabaseName()) | ||
.setParams(getParams()) |
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.
👍 nice
} | ||
|
||
@Override | ||
public void use() { | ||
} | ||
|
||
// GN added this to get a connection on the database engine without a database name | ||
public void executeStatementOnEngine(String sqlStatement) { |
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.
👍 good
@@ -24,6 +24,7 @@ public static void initialize() { | |||
executeSqlFile("courses.populate-table.sql"); | |||
executeSqlFile("students.create-table.sql"); | |||
executeSqlFile("students.populate-table.sql"); | |||
executeSqlFile("studentcourse.create-table.sql"); //GN added for many-to-many relationship between Student and Course |
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.
👍
} | ||
|
||
@Override | ||
public Boolean validateStudent(String studentEmail, String password) { | ||
return null; | ||
String query = "SELECT * FROM Student WHERE email = \'" + |
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.
nice implementation 👍
@@ -77,6 +82,7 @@ public String toString() { | |||
.append(hasPortBeenSet ? portNumber : "") | |||
.append("/") | |||
.append(databaseName != null ? databaseName : "") | |||
.append(params != null ? params : "") |
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.
👍
…s, one containing valid students, and one containing invalid students.
I added two parameterized tests using CSV files to GetStudentByEmailTest. Will add a few more ... |
…ecuteStatement to proreturn an int, which is the return type of JDBC executeUpdate that needs to be propagated up to any method calling executeStatement.
…processing of the .sql scripts in the resources folder.