Task 1-100%, Task 2-80% #2

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.

Sign up for GitHub

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

Jump to bottom

Open

VynnykVV wants to merge 1 commit into ChangeRequest:master from VynnykVV:master

VynnykVV commented Dec 8, 2016

1.Employees-done
2.Fraction_Number->halfway
-you can divide by 0
-no objects ONE/TEN


          1.Employees-done

70b184d

2.Fraction_Number->halfway
-you can divide by 0
-no objects ONE/TEN

didva self-assigned this

didva requested changes

View reviewed changes

Contributor

didva left a comment

Please add 3rd task, fix comments and format your code.

src/main/java/school/lemon/changerequest/java/Fraction_Number/FractionNumber.java

+                  public FractionNumber() {
+                  }
+                  public FractionNumber(int Numerator, int Denominator) {

Contributor

didva Dec 10, 2016

Please read java convention. All variables, fields and method arguments should be named on lowerCamelCase.

src/main/java/school/lemon/changerequest/java/Fraction_Number/FractionNumber.java



		final int def = 1;
		protected int Numerator;

Contributor

didva Dec 10, 2016

Please move these fields on the top of the class.

src/main/java/school/lemon/changerequest/java/Fraction_Number/FractionNumber.java

+                  protected int Numerator;
+                  protected int Denominator;
+                  int a = Numerator;

Contributor

didva Dec 10, 2016

Why do you need variables a, b?

src/main/java/school/lemon/changerequest/java/Fraction_Number/FractionNumber.java

+                      Ad = Ad * aN;
+                      An = An * aD;
+                      FractionNumber b = new FractionNumber(An, Ad);
+                      b.Normalized();

Contributor

didva Dec 10, 2016

Normalization should be done right on object creation. So please move it to constructor.

src/main/java/school/lemon/changerequest/java/Fraction_Number/FractionNumber.java

+                  public FractionNumber(int Numerator, int Denominator) {
+                      this.Numerator = Numerator;
+                      if(Denominator==0){System.out.println("WRONG");}

Contributor

didva Dec 10, 2016

System.out.println("WRONG"); is not the case. Please set denominator to 1 or throw exception.

src/main/java/school/lemon/changerequest/java/Employees/Employees.java

+              /**
+               * Created by User on 05.12.2016.
+               */
+              public class Employees {

Contributor

didva Dec 10, 2016

Employee is most likely better name for this class.

src/main/java/school/lemon/changerequest/java/Employees/Employees.java




		protected String name;

Contributor

didva Dec 10, 2016

Please move fields on the top of the class.

src/main/java/school/lemon/changerequest/java/Employees/Employees.java

+               * Created by User on 05.12.2016.
+               */
+              public class Employees {
+                  public Employees(String nameE,int workHoursE){

Contributor

didva Dec 10, 2016

Not sure it's a good way to pass worked hours via constructor. I'd prefer add method like work(int hours) or simple setWorkedHours(int hours).

src/main/java/school/lemon/changerequest/java/Employees/Accountant.java

+               * Created by User on 05.12.2016.
+               */
+              public class Accountant extends Employees {
+                  private static int countAccountant;

Contributor

didva Dec 10, 2016

There is no need to count all objects that were created.

src/main/java/school/lemon/changerequest/java/Employees/Accountant.java

+                  public Accountant(String nameE, int workHoursE) {
+                      super(nameE, workHoursE);
+                      countAccountant++;
+                      Manager.SalaryOverAll=Manager.SalaryOverAll+getSalary();

Contributor

didva Dec 10, 2016

Accountant should contain array of employees and calculate their salary instead of static fields.
You should avoid static fields/methods everywhere it possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet