Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements a lab assignment for practicing Java classes, inheritance, and file I/O. The solution creates an Employee class hierarchy with validation, generates employee data, and writes it to a file.
Changes:
- Added
Employeeclass with properties (name, email, age, salary) and validation in setters - Added
Internsubclass extendingEmployeewith a salary cap of 20,000 - Implemented
Mainclass to create 10 employees and write them toemployees.txt
Reviewed changes
Copilot reviewed 6 out of 17 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| LabProject/src/main/java/org/example/Employee.java | Core Employee class with getters/setters and validation logic |
| LabProject/src/main/java/org/example/Intern.java | Intern subclass with salary cap enforcement |
| LabProject/src/main/java/org/example/Main.java | Main application that creates employees and writes to file |
| LabProject/pom.xml | Maven project configuration |
| employees.txt | Generated output file containing employee data |
| README.md | Added solution notes and run instructions |
| LabProject/target/classes/* | Compiled class files (should be gitignored) |
| .idea/* | IDE configuration files (already partially gitignored) |
Files not reviewed (8)
- .idea/.gitignore: Language not supported
- .idea/compiler.xml: Language not supported
- .idea/encodings.xml: Language not supported
- .idea/jarRepositories.xml: Language not supported
- .idea/lab-java-standard-input-and-classes.iml: Language not supported
- .idea/misc.xml: Language not supported
- .idea/modules.xml: Language not supported
- .idea/vcs.xml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this.name = name; | ||
| this.age = age; | ||
| this.email = email; | ||
| this.salary = salary; |
There was a problem hiding this comment.
The constructor does not validate the input parameters before assigning them to the fields. This means invalid values can be set during object construction, bypassing the validation logic in the setters. For example, an Employee can be created with age less than 18 or negative salary, which contradicts the validation in setAge and setSalary methods. Consider either using the setter methods in the constructor or duplicating the validation logic.
| this.name = name; | |
| this.age = age; | |
| this.email = email; | |
| this.salary = salary; | |
| setName(name); | |
| setEmail(email); | |
| setAge(age); | |
| setSalary(salary); |
| if(name.length()<1){ | ||
| System.out.println("Invalid Name!"); | ||
| } | ||
| else { | ||
| this.name=name; | ||
| } | ||
| } | ||
| public String getEmail(){ | ||
| return email; | ||
| } | ||
| public void setEmail(String email){ | ||
| if(email.contains("@")){ | ||
| this.email = email; | ||
| } | ||
| else{ | ||
| System.out.println("Invalid Email"); | ||
| } | ||
| } | ||
| public int getAge() { | ||
| return age; | ||
| } | ||
| public void setAge(int age){ | ||
| if(age < 18 ) | ||
| {System.out.println("Invalid Age for being Employee.");} | ||
| else { | ||
| this.age=age; | ||
| } | ||
| } | ||
| public double getSalary() { | ||
| return salary; | ||
| } | ||
| public void setSalary(double salary){ | ||
| if(salary<0) | ||
| {System.out.println("Salary cannot be negative.");} | ||
| else { | ||
| this.salary=salary; | ||
| } |
There was a problem hiding this comment.
The validation logic prints error messages but doesn't prevent invalid values from being accepted. When validation fails, the method should either throw an exception or return early without updating the field. Currently, if name length is less than 1, an error message is printed but the else block is not triggered, leaving the field unchanged. This inconsistent behavior can be confusing. Consider using a consistent validation approach across all setters.
| if(name.length()<1){ | |
| System.out.println("Invalid Name!"); | |
| } | |
| else { | |
| this.name=name; | |
| } | |
| } | |
| public String getEmail(){ | |
| return email; | |
| } | |
| public void setEmail(String email){ | |
| if(email.contains("@")){ | |
| this.email = email; | |
| } | |
| else{ | |
| System.out.println("Invalid Email"); | |
| } | |
| } | |
| public int getAge() { | |
| return age; | |
| } | |
| public void setAge(int age){ | |
| if(age < 18 ) | |
| {System.out.println("Invalid Age for being Employee.");} | |
| else { | |
| this.age=age; | |
| } | |
| } | |
| public double getSalary() { | |
| return salary; | |
| } | |
| public void setSalary(double salary){ | |
| if(salary<0) | |
| {System.out.println("Salary cannot be negative.");} | |
| else { | |
| this.salary=salary; | |
| } | |
| if (name.length() < 1) { | |
| System.out.println("Invalid Name!"); | |
| return; | |
| } | |
| this.name = name; | |
| } | |
| public String getEmail(){ | |
| return email; | |
| } | |
| public void setEmail(String email){ | |
| if (!email.contains("@")) { | |
| System.out.println("Invalid Email"); | |
| return; | |
| } | |
| this.email = email; | |
| } | |
| public int getAge() { | |
| return age; | |
| } | |
| public void setAge(int age){ | |
| if (age < 18) { | |
| System.out.println("Invalid Age for being Employee."); | |
| return; | |
| } | |
| this.age = age; | |
| } | |
| public double getSalary() { | |
| return salary; | |
| } | |
| public void setSalary(double salary){ | |
| if (salary < 0) { | |
| System.out.println("Salary cannot be negative."); | |
| return; | |
| } | |
| this.salary = salary; |
| public void setEmail(String email){ | ||
| if(email.contains("@")){ | ||
| this.email = email; | ||
| } | ||
| else{ | ||
| System.out.println("Invalid Email"); | ||
| } | ||
| } |
There was a problem hiding this comment.
The email validation only checks if the email contains an "@" symbol, which is insufficient. This allows invalid emails like "@@", "@test", or "test@" to be accepted. Consider using a more robust email validation pattern or at minimum checking that "@" is not at the beginning or end of the string.
| package org.example; | ||
|
|
||
| public class Intern extends Employee { | ||
| private boolean isIntern; |
There was a problem hiding this comment.
The isIntern field is declared but never used anywhere in the class. This field serves no purpose and should either be removed or properly utilized if it was intended to track intern status.
| private boolean isIntern; |
| super(name, email, age, salary); | ||
| if (salary > LIMIT) { |
There was a problem hiding this comment.
The constructor calls super() which invokes the Employee constructor, passing the salary parameter. However, the Employee constructor does not validate the salary, allowing an Intern to be created with salary greater than LIMIT. The validation on line 9 happens after the parent constructor has already set the invalid salary value in the parent class field. This is a logic error - the validation should happen before calling super(), or the parent constructor should be modified to validate.
| super(name, email, age, salary); | |
| if (salary > LIMIT) { | |
| boolean invalidSalary = salary > LIMIT; | |
| super(name, email, age, invalidSalary ? 0.0 : salary); | |
| if (invalidSalary) { |
| ## Solution Notes | ||
|
|
||
| - The `Employee` class includes getters/setters and a `toFileLine()` helper for output formatting. | ||
| - The `Intern` class enforces a salary cap of 20000 in both constructor and setter. |
There was a problem hiding this comment.
The documentation states that the Intern class enforces a salary cap of 20000 in the constructor, but this is misleading. Due to the bug in the Intern constructor where super() is called before validation, the salary cap is not actually enforced during construction - the parent class sets the salary value before the validation check throws an exception. This documentation should be updated once the constructor issue is fixed.
| - The `Intern` class enforces a salary cap of 20000 in both constructor and setter. | |
| - The `Intern` class should enforce a salary cap of 20000; ensure you validate salaries (including in the constructor, before calling `super(...)`) and in the setter. |
| throw new IllegalArgumentException("Intern salary can not be more than " + LIMIT); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void setSalary(double salary){ | ||
| if (salary > LIMIT) { | ||
| throw new IllegalArgumentException("Intern salary can not be more than " + LIMIT); |
There was a problem hiding this comment.
Grammar error: "can not" should be written as one word "cannot" in formal English. The same error appears on line 17.
| throw new IllegalArgumentException("Intern salary can not be more than " + LIMIT); | |
| } | |
| } | |
| @Override | |
| public void setSalary(double salary){ | |
| if (salary > LIMIT) { | |
| throw new IllegalArgumentException("Intern salary can not be more than " + LIMIT); | |
| throw new IllegalArgumentException("Intern salary cannot be more than " + LIMIT); | |
| } | |
| } | |
| @Override | |
| public void setSalary(double salary){ | |
| if (salary > LIMIT) { | |
| throw new IllegalArgumentException("Intern salary cannot be more than " + LIMIT); |
| System.out.println("Invalid Name!"); | ||
| } | ||
| else { | ||
| this.name=name; |
There was a problem hiding this comment.
Inconsistent formatting: this line uses inconsistent spacing around the assignment operator compared to other assignments in the class. Line 22 has "this.name=name;" without spaces, while line 30 has "this.email = email;" with spaces. Consistent formatting improves code readability.
| this.name=name; | |
| this.name = name; |
| if(age < 18 ) | ||
| {System.out.println("Invalid Age for being Employee.");} | ||
| else { | ||
| this.age=age; | ||
| } | ||
| } | ||
| public double getSalary() { | ||
| return salary; | ||
| } | ||
| public void setSalary(double salary){ | ||
| if(salary<0) | ||
| {System.out.println("Salary cannot be negative.");} | ||
| else { | ||
| this.salary=salary; |
There was a problem hiding this comment.
Inconsistent brace placement: the if-else statement has the opening brace on the same line as the condition, while the else clause on line 52 has inconsistent formatting. Line 40-41 places the if condition and opening brace on the same line but the statement on a new line without braces, while line 50-51 does the same. For consistency and to prevent bugs, always use braces for if-else blocks.
| if(age < 18 ) | |
| {System.out.println("Invalid Age for being Employee.");} | |
| else { | |
| this.age=age; | |
| } | |
| } | |
| public double getSalary() { | |
| return salary; | |
| } | |
| public void setSalary(double salary){ | |
| if(salary<0) | |
| {System.out.println("Salary cannot be negative.");} | |
| else { | |
| this.salary=salary; | |
| if (age < 18) { | |
| System.out.println("Invalid Age for being Employee."); | |
| } else { | |
| this.age = age; | |
| } | |
| } | |
| public double getSalary() { | |
| return salary; | |
| } | |
| public void setSalary(double salary){ | |
| if (salary < 0) { | |
| System.out.println("Salary cannot be negative."); | |
| } else { | |
| this.salary = salary; |
| throw new IllegalArgumentException("Intern salary can not be more than " + LIMIT); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void setSalary(double salary){ | ||
| if (salary > LIMIT) { | ||
| throw new IllegalArgumentException("Intern salary can not be more than " + LIMIT); |
There was a problem hiding this comment.
Grammar error: "can not" should be written as one word "cannot" in formal English. The same error appears on line 10.
| throw new IllegalArgumentException("Intern salary can not be more than " + LIMIT); | |
| } | |
| } | |
| @Override | |
| public void setSalary(double salary){ | |
| if (salary > LIMIT) { | |
| throw new IllegalArgumentException("Intern salary can not be more than " + LIMIT); | |
| throw new IllegalArgumentException("Intern salary cannot be more than " + LIMIT); | |
| } | |
| } | |
| @Override | |
| public void setSalary(double salary){ | |
| if (salary > LIMIT) { | |
| throw new IllegalArgumentException("Intern salary cannot be more than " + LIMIT); |
No description provided.