13 Replies - 665 Views - Last Post: 27 December 2011 - 11:33 AM Rate Topic: -----

#1 Vampiricx3  Icon User is offline

  • D.I.C Head

Reputation: -2
  • View blog
  • Posts: 85
  • Joined: 01-December 11

How can I improve this?

Posted 27 December 2011 - 01:30 AM

hey guys, I'm currently trying to get my Java skill higher than the sky, and I've made up this program to read in objects from an array, divide it up into titles, salary over 100k, trainees, etc. There is also an output that is read in by a C++ program and then sorted out as the Java program is. Can anyone tell me some shit to do to improve on my skills? My source code is below!
import java.io.*;
import java.util.*; 

public class Working {

	public Working() {
		String read;
	    //Employee[] employees = new Employee[10];
	    ArrayList<Employee> employees = new ArrayList<Employee>();
	    
	    	employees.add(new Employee(1, "Taylah Jayne", 2000, 50000.00, 5));
	    	employees.add(new Employee(2, "Hurp Hurp", 1981, 1000000.00, 3));
	    	employees.add(new Employee(3, "Hurp Durp", 2011, 10000.00, 1));
	    	employees.add(new Employee(4, "4chan l00lz", 2004, 0.00, 2));
	    	employees.add(new Employee(5, "m00t harz", 2004, 200000.00, 3));
	    	employees.add(new Employee(6, "Derpina z0r", 2009, 15000.00, 1));
	    	employees.add(new Employee(7, "Nathan Kreider", 2000, 500000.00, 5));
	    	employees.add(new Employee(8, "Durp Dur", 2000, 50000.00, 4));
	    	employees.add(new Employee(9, "Durp Hurp", 1821, 180912.00, 5));
	    	employees.add(new Employee(10, "Le Durp", 1921, 1920101.00, 2));
	    	employees.add(new Employee(11, "Nathan Kreider", 2000, 500000.00, 5));
	    	employees.add(new Employee(12, "What Durp", 1200, 500000.00, 1));
	    	employees.add(new Employee(13, "Oh No", 1800, 120000.00, 3));
	    	employees.add(new Employee(14, "Waddup Bruh", 2012, 180000.00, 1));
	    	employees.add(new Employee(15, "Jai Mason", 2007, 12000.00, 3));
	    	
	    	try {
	             FileWriter out = new FileWriter(new File("src/main.txt"));
	             FileWriter trainee = new FileWriter(new File("src/trainee.txt"));
	             FileWriter manager = new FileWriter(new File("src/manager.txt"));
	             
	             FileWriter cOut = new FileWriter(new File("src/c++/cOut.txt"));
	             FileWriter cTrainee = new FileWriter(new File("src/c++/cTrainee.txt"));
	             FileWriter cManager = new FileWriter(new File("src/c++/cManager.txt"));
	    
	        out.write("All Employees");
	    	System.out.println("All Employees");
	    for(int i = 0; i < employees.size(); i++) {
	    	out.write("\n" + employees.get(i).getData());
	    	cOut.write(employees.get(i).cOutput() + "\n");
	    			  employees.get(i).print();
	    }
	    	out.write("\n\nEmployees who have been working here for 10 years or more.");
	    	System.out.println("\nEmployees who have been working here for 10 years or more.");
	    for(int i = 0; i < employees.size(); i++) {
	    	if(employees.get(i).getHireYear() <= 2011 - 10) {
	    	//if(employees().getHireYear() <= 2011 - 10) {
	    			out.write("\n" + employees.get(i).getData());
	    					  employees.get(i).print();
					//employees().print();
	    	}
	  }
	    	out.write("\n\nEmployees with salary greater than 100000.00.");
	    	System.out.println("\nEmployees with salary greater than 100000.00.");
	    for(int i = 0; i < employees.size(); i++ ) {
	    	if(employees.get(i).getSalary() > 100000.00)
	    	//if(employees(i).getSalary() > 100000.00)
	    			out.write("\n" + employees.get(i).getData());
	    					  employees.get(i).print();
	    		//employees(i).print();
	    }
	    	System.out.println("\nManagers");
	    for(int i = 0; i < employees.size(); i++) {
	    	if(employees.get(i).getLeTitle() >= 4) {
	    		manager.write("\n" + employees.get(i).getData());
	    		cManager.write(employees.get(i).cOutput() + "\n");
	    				  employees.get(i).print();
	    	}
	    }
	    	out.write("\n\nTrainee's");
	    	System.out.println("\nTrainees");
	    for(int i = 0; i < employees.size(); i++) {
	    	if(employees.get(i).getLeTitle() == 1) {
	    		out.write("\n" + employees.get(i).getData());
	    		trainee.write("\n" + employees.get(i).getData());
	    		cTrainee.write(employees.get(i).cOutput() + "\n");
	    				  employees.get(i).print();
	    	}
	    }
	    	cManager.close();
	    	cTrainee.close();
	    	cOut.close();
	    	
	    	manager.close();
	 		trainee.close();
			out.close();
	    	} catch(IOException e) {
	            System.out.println("There was a problem:" + e);
	        }
	}

  public static void main(String[] args) {
    new Working();
    }

  public class Employee {
    private int id;
    private String name;
    private int hireYear;
    private double salary;
    private int title;

    public Employee() {
      id = 1;
      name = "Nathan Kreider";
      hireYear = 2000;
      salary = 50000.00;
      title = 5;
    }

    public Employee(int newId, String newName, int newHireYear, double newSalary, int newTitle) {
      id = newId;
      name = newName;
      hireYear = newHireYear;
      salary = newSalary;
      title = newTitle;
    }

    public String getData() {
      return "  [" + id + "] Name: " + name + " | Title: " + getTitle() + " | Hired in: " + hireYear + " | Salary: $" + salary;
    }
    
    public void print() {
    	System.out.println("   [" + id + "] Name: " + name + " | Title: " + getTitle() + " | Hired in: " + hireYear + " | Salary: $" + salary);
    }
    
    public String cOutput() {
    	return id + " " + name + " " + getTitle() + " " + hireYear + " " + salary;
    }
    
    public void setHireYear(int hire1) {
	    hireYear = hire1;
	}
	
	public int getHireYear() {
	    return hireYear;
	}
	
	public double getSalary() {
		return salary;
	}
	
	public String getTitle() {
	  String Title = "";
	  
		if(getHireYear() > 2010) {
			Title = "Trainee";
		} if(title == 1) {
			Title = "Trainee";
		} if(title == 2) {
			Title = "Employee";
		} if(title == 3) {
			Title = "Senior";
		} if(title == 4) {
			Title = "Manager";
		} if(title == 5) {
			Title = "Owner";
		}
		
		return Title;
	}
	
	public int getLeTitle() {
		return title;
	}
  }
}


Is This A Good Question/Topic? 0
  • +

Replies To: How can I improve this?

#2 TheCompBoy  Icon User is offline

  • D.I.C Regular

Reputation: 11
  • View blog
  • Posts: 314
  • Joined: 21-April 11

Re: How can I improve this?

Posted 27 December 2011 - 03:58 AM

Its a good habbit to always put comments in your code, If you work with other people it may help them to understand your code.
Was This Post Helpful? 0
  • +
  • -

#3 GregBrannon  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 2203
  • View blog
  • Posts: 5,235
  • Joined: 10-September 10

Re: How can I improve this?

Posted 27 December 2011 - 05:25 AM

You've made an evolutionary step by moving most of the code from the main() method to the constructor, but you now have a program where nearly its entire function is in the constructor. If this were a class that you wanted to use twice, there are no options for creating something different. You cureently have no options to reuse this code.

For example, let's say you're an independent consultant for multiple companies, and you used this program to keep track of each company's employees. In that case, you'd want to create a separate instance of Working by passing the employee info in an array, file name, or database to the Working constructor:

Working company2 = new Working( company2Employees );

Where in this case company2Employees is an array that contains all of the data for company2's employees.

You'd then want the constructor to call a method that moved the employee data into an arraylist of Employee objects. Rather than having main pass an array to your constructor for this exercise, you could define the array elsewhere and create a method that creates the arraylist from the data in the array.

You could then do similarly for the other functions currently done in your constructor. Ultimately, the constructor would do the minimum necessary to create a new Working object, and most everything else currently cone in the constructor would be moved to instance methods. You would then exercise the program using a separate driver or test class.

This post has been edited by GregBrannon: 27 December 2011 - 05:34 AM

Was This Post Helpful? 0
  • +
  • -

#4 Vampiricx3  Icon User is offline

  • D.I.C Head

Reputation: -2
  • View blog
  • Posts: 85
  • Joined: 01-December 11

Re: How can I improve this?

Posted 27 December 2011 - 06:31 AM

View PostTheCompBoy, on 27 December 2011 - 03:58 AM, said:

Its a good habbit to always put comments in your code, If you work with other people it may help them to understand your code.

This is actually a pretty good idea, although right now the work I do is by myself, but this would be a very good thing to get into the habit for later on in life when I work with a group of people. Thank you!

View PostGregBrannon, on 27 December 2011 - 05:25 AM, said:

You've made an evolutionary step by moving most of the code from the main() method to the constructor, but you now have a program where nearly its entire function is in the constructor. If this were a class that you wanted to use twice, there are no options for creating something different. You cureently have no options to reuse this code.

For example, let's say you're an independent consultant for multiple companies, and you used this program to keep track of each company's employees. In that case, you'd want to create a separate instance of Working by passing the employee info in an array, file name, or database to the Working constructor:

Working company2 = new Working( company2Employees );

Where in this case company2Employees is an array that contains all of the data for company2's employees.

You'd then want the constructor to call a method that moved the employee data into an arraylist of Employee objects. Rather than having main pass an array to your constructor for this exercise, you could define the array elsewhere and create a method that creates the arraylist from the data in the array.

You could then do similarly for the other functions currently done in your constructor. Ultimately, the constructor would do the minimum necessary to create a new Working object, and most everything else currently cone in the constructor would be moved to instance methods. You would then exercise the program using a separate driver or test class.

This to me, sounds really good to do in theory, and to put it into action would be a big step for me, haha. I understand it all, I just have no clue on how I would implement this into my Java class. Would I need to make a new constructor?
Was This Post Helpful? 0
  • +
  • -

#5 GregBrannon  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 2203
  • View blog
  • Posts: 5,235
  • Joined: 10-September 10

Re: How can I improve this?

Posted 27 December 2011 - 06:47 AM

No, to incrementally implement my suggestion, you would not need a new or different constructor. Obviously, your current constructor would change. On the whole, incorporating my suggestion may seem like a daunting task, but taken in small steps, implementation would involve a lot more moving of existing code than rewriting new code.

And it's just a suggestion. You may have had something entirely different in mind when you asked "How can I improve this?" The paths to improving one's Java skill "higher than the sky" are many.
Was This Post Helpful? 0
  • +
  • -

#6 baavgai  Icon User is offline

  • Dreaming Coder
  • member icon

Reputation: 5879
  • View blog
  • Posts: 12,757
  • Joined: 16-October 07

Re: How can I improve this?

Posted 27 December 2011 - 07:04 AM

Here's an example of some cleanup.

public class Working {
	// employees belongs to class
	private List<Employee> employees;

	// helper method
	// you could also have one that auto incremented the newId...
	private void addEmployee(int newId, String newName, int newHireYear, double newSalary, int newTitle) {
		this.employees.add(new Employee(newId, newName, newHireYear, newSalary, newTitle));
	}


	// your constructor
	// here we initialize your employees list
	public Working() {
		employees = new ArrayList<Employee>();
		addEmployee(1, "Taylah Jayne", 2000, 50000.00, 5);
		addEmployee(2, "Hurp Hurp", 1981, 1000000.00, 3);
		addEmployee(3, "Hurp Durp", 2011, 10000.00, 1);
		addEmployee(4, "4chan l00lz", 2004, 0.00, 2);
		addEmployee(5, "m00t harz", 2004, 200000.00, 3);
		addEmployee(6, "Derpina z0r", 2009, 15000.00, 1);
		addEmployee(7, "Nathan Kreider", 2000, 500000.00, 5);
		addEmployee(8, "Durp Dur", 2000, 50000.00, 4);
		addEmployee(9, "Durp Hurp", 1821, 180912.00, 5);
		addEmployee(10, "Le Durp", 1921, 1920101.00, 2);
		addEmployee(11, "Nathan Kreider", 2000, 500000.00, 5);
		addEmployee(12, "What Durp", 1200, 500000.00, 1);
		addEmployee(13, "Oh No", 1800, 120000.00, 3);
		addEmployee(14, "Waddup Bruh", 2012, 180000.00, 1);
		addEmployee(15, "Jai Mason", 2007, 12000.00, 3);
	}

	// another helper method
	private void writeEmployee(Writer out, Writer cOut, Employee emp) throws IOException {
		if (out!=null) { out.write("\n" + emp.getData()); }
		if (cOut!=null) { cOut.write(emp.cOutput() + "\n"); }
		System.out.println(emp.getData());
	}

	private void writeAll(Writer out, Writer cOut) throws IOException { /* your code here */ }
	private void writeTenYear(Writer out, Writer cOut) throws IOException { /* your code here */ }
	private void writeHighSal(Writer out, Writer cOut) throws IOException { /* your code here */ }
	private void writeManagers(Writer out, Writer cOut) throws IOException { /* your code here */ }
	private void writeTrainees(Writer out, Writer cOut) throws IOException { /* your code here */ }

	// grouped because they share writers
	private void writeOther(Writer out, Writer cOut) throws IOException {
		writeAll(out, cOut);
		writeTenYear(out, null);
		writeHighSal(out, null);
	}

	private Writer getFileWriter(String filename) throws IOException {
		return new FileWriter(new File(filename));
	}

	private void writeTrainees() throws IOException {
		Writer out = getFileWriter("src/trainee.txt");
		Writer cOut = getFileWriter("src/c++/cTrainee.txt");
		writeTrainees(out, cOut);
		out.close();
		cOut.close();
	}

	private void writeManagers() throws IOException {
		Writer out = getFileWriter("src/manager.txt");
		Writer cOut = getFileWriter("src/c++/cManager.txt");
		writeManagers(out, cOut);
		out.close();
		cOut.close();
	}

	private void writeOther() throws IOException {
		Writer out = getFileWriter("src/main.txt");
		Writer cOut = getFileWriter("src/c++/cOut.txt");
		writeManagers(out, cOut);
		out.close();
		cOut.close();
	}

	public void writeAll() {
		try {
			writeOther();
			writeManagers();
			writeTrainees();
	    } catch(IOException e) {
			System.out.println("There was a problem:" + e);
		}
	}

	public void writeAllNoFiles() {
		try {
			writeOther(null,null);
			writeManagers(null,null);
			writeTrainees(null,null);
	    } catch(IOException e) {
			System.out.println("There was a problem:" + e);
		}
	}


  public static void main(String[] args) {
    // new Working().writeAll();
	new Working().writeAllNoFiles();

    }
}



There are tons of ways to organize this, of course. You could make a reporting class with a title and filter that takes file locations and has a runReport method that takes List<Employee>.

Most coding is about organization. And, importantly, repetition avoidance. If you have a large block of code in a method, consider how you might break it up. See if you have code that repeats, that you could put in a method.

Hope this helps.
Was This Post Helpful? 0
  • +
  • -

#7 Vampiricx3  Icon User is offline

  • D.I.C Head

Reputation: -2
  • View blog
  • Posts: 85
  • Joined: 01-December 11

Re: How can I improve this?

Posted 27 December 2011 - 07:25 AM

View PostGregBrannon, on 27 December 2011 - 06:47 AM, said:

No, to incrementally implement my suggestion, you would not need a new or different constructor. Obviously, your current constructor would change. On the whole, incorporating my suggestion may seem like a daunting task, but taken in small steps, implementation would involve a lot more moving of existing code than rewriting new code.

And it's just a suggestion. You may have had something entirely different in mind when you asked "How can I improve this?" The paths to improving one's Java skill "higher than the sky" are many.

Daunting is a bit of an understatement for me! Haha. I'm fairly new to Java, and right now all of this is very alien to me.

View Postbaavgai, on 27 December 2011 - 07:04 AM, said:

Here's an example of some cleanup.

public class Working {
	// employees belongs to class
	private List<Employee> employees;

	// helper method
	// you could also have one that auto incremented the newId...
	private void addEmployee(int newId, String newName, int newHireYear, double newSalary, int newTitle) {
		this.employees.add(new Employee(newId, newName, newHireYear, newSalary, newTitle));
	}


	// your constructor
	// here we initialize your employees list
	public Working() {
		employees = new ArrayList<Employee>();
		addEmployee(1, "Taylah Jayne", 2000, 50000.00, 5);
		addEmployee(2, "Hurp Hurp", 1981, 1000000.00, 3);
		addEmployee(3, "Hurp Durp", 2011, 10000.00, 1);
		addEmployee(4, "4chan l00lz", 2004, 0.00, 2);
		addEmployee(5, "m00t harz", 2004, 200000.00, 3);
		addEmployee(6, "Derpina z0r", 2009, 15000.00, 1);
		addEmployee(7, "Nathan Kreider", 2000, 500000.00, 5);
		addEmployee(8, "Durp Dur", 2000, 50000.00, 4);
		addEmployee(9, "Durp Hurp", 1821, 180912.00, 5);
		addEmployee(10, "Le Durp", 1921, 1920101.00, 2);
		addEmployee(11, "Nathan Kreider", 2000, 500000.00, 5);
		addEmployee(12, "What Durp", 1200, 500000.00, 1);
		addEmployee(13, "Oh No", 1800, 120000.00, 3);
		addEmployee(14, "Waddup Bruh", 2012, 180000.00, 1);
		addEmployee(15, "Jai Mason", 2007, 12000.00, 3);
	}

	// another helper method
	private void writeEmployee(Writer out, Writer cOut, Employee emp) throws IOException {
		if (out!=null) { out.write("\n" + emp.getData()); }
		if (cOut!=null) { cOut.write(emp.cOutput() + "\n"); }
		System.out.println(emp.getData());
	}

	private void writeAll(Writer out, Writer cOut) throws IOException { /* your code here */ }
	private void writeTenYear(Writer out, Writer cOut) throws IOException { /* your code here */ }
	private void writeHighSal(Writer out, Writer cOut) throws IOException { /* your code here */ }
	private void writeManagers(Writer out, Writer cOut) throws IOException { /* your code here */ }
	private void writeTrainees(Writer out, Writer cOut) throws IOException { /* your code here */ }

	// grouped because they share writers
	private void writeOther(Writer out, Writer cOut) throws IOException {
		writeAll(out, cOut);
		writeTenYear(out, null);
		writeHighSal(out, null);
	}

	private Writer getFileWriter(String filename) throws IOException {
		return new FileWriter(new File(filename));
	}

	private void writeTrainees() throws IOException {
		Writer out = getFileWriter("src/trainee.txt");
		Writer cOut = getFileWriter("src/c++/cTrainee.txt");
		writeTrainees(out, cOut);
		out.close();
		cOut.close();
	}

	private void writeManagers() throws IOException {
		Writer out = getFileWriter("src/manager.txt");
		Writer cOut = getFileWriter("src/c++/cManager.txt");
		writeManagers(out, cOut);
		out.close();
		cOut.close();
	}

	private void writeOther() throws IOException {
		Writer out = getFileWriter("src/main.txt");
		Writer cOut = getFileWriter("src/c++/cOut.txt");
		writeManagers(out, cOut);
		out.close();
		cOut.close();
	}

	public void writeAll() {
		try {
			writeOther();
			writeManagers();
			writeTrainees();
	    } catch(IOException e) {
			System.out.println("There was a problem:" + e);
		}
	}

	public void writeAllNoFiles() {
		try {
			writeOther(null,null);
			writeManagers(null,null);
			writeTrainees(null,null);
	    } catch(IOException e) {
			System.out.println("There was a problem:" + e);
		}
	}


  public static void main(String[] args) {
    // new Working().writeAll();
	new Working().writeAllNoFiles();

    }
}



There are tons of ways to organize this, of course. You could make a reporting class with a title and filter that takes file locations and has a runReport method that takes List<Employee>.

Most coding is about organization. And, importantly, repetition avoidance. If you have a large block of code in a method, consider how you might break it up. See if you have code that repeats, that you could put in a method.

Hope this helps.

This is very quite different to what I had in mind with cleaning up, haha. I'll probably just copy and paste this code to my existing source code, if it would work. Going to analyze this to kingdom come to improve on my coding. Thank you, good sir!
Was This Post Helpful? 0
  • +
  • -

#8 jon.kiparsky  Icon User is online

  • Pancakes!
  • member icon


Reputation: 7875
  • View blog
  • Posts: 13,357
  • Joined: 19-March 11

Re: How can I improve this?

Posted 27 December 2011 - 07:49 AM

View PostTheCompBoy, on 27 December 2011 - 05:58 AM, said:

Its a good habbit to always put comments in your code, If you work with other people it may help them to understand your code.


Yes and no...
It's a good idea to include documentation comments explaining what this program is meant to accomplish, and any constraints that might not be obvious. This helps me, the outside reader, orient myself to the code. Each method should also have some explanation of what it's meant to do, as well, again including non-obvious restrictions.
However, you should always write your code such that comments in the code are not necessary. Very occasionally you might find a need to explain a particular step, or to flag something as a potential issue, but if you have time to write comments in the body of the code, I'd prefer you make the code more readable.

Comments in code are great for pedagogical purposes, but if it's working code it's common for updates to the code to not be reflected in the comments; this is why many programmers will actually strip out in-line comments when they go to look at a new codebase.
Was This Post Helpful? 2
  • +
  • -

#9 blackcompe  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 1155
  • View blog
  • Posts: 2,535
  • Joined: 05-May 05

Re: How can I improve this?

Posted 27 December 2011 - 08:12 AM

In addition to breaking your code into intelligently-named single-responsibility methods as shown by baavgai, how about naming the class according to what is represents. Working tells me nothing about what the class does or represents. How about Company? Also, remember that identifiers (with the of exception boolean statuses) should be nouns or action verbs. E.g.

Company, Employee, addEmployee, writeEmployee, isInTraining (the exception).

If your method is longer than 15-20 lines it's probably too long. It's probably not cohesive. This isn't always true though, a sorting algorithm implementation is one example. To be cohesive is to stick to accomplishing a single thing. Yeah, you end with a lot more code, but readability is more important. Try to decompose it functionally. baavgai's code is clean, cohesive, and has minimal coupling (notice how he sent the Writer to method, instead of declaring it as a local variable).

private void writeAll(Writer out, Writer cOut) throws IOException


Your Employee class looks good. If you really want to become the best developer you can, read some books on refactoring and see how much better you code looks. I think there's a refactoring tutorial on this site.

This post has been edited by blackcompe: 27 December 2011 - 08:13 AM

Was This Post Helpful? 2
  • +
  • -

#10 jon.kiparsky  Icon User is online

  • Pancakes!
  • member icon


Reputation: 7875
  • View blog
  • Posts: 13,357
  • Joined: 19-March 11

Re: How can I improve this?

Posted 27 December 2011 - 08:23 AM

View PostVampiricx3, on 27 December 2011 - 03:30 AM, said:

Can anyone tell me some shit to do to improve on my skills?


One obvious suggestion would be to look into the Properties class. This will allow you to externalize your data. You absolutely do not want this:

	    	employees.add(new Employee(1, "Taylah Jayne", 2000, 50000.00, 5));
	    	employees.add(new Employee(2, "Hurp Hurp", 1981, 1000000.00, 3));
	    	employees.add(new Employee(3, "Hurp Durp", 2011, 10000.00, 1));
	    	employees.add(new Employee(4, "4chan l00lz", 2004, 0.00, 2));
	    	employees.add(new Employee(5, "m00t harz", 2004, 200000.00, 3));
	    	employees.add(new Employee(6, "Derpina z0r", 2009, 15000.00, 1));
	    	employees.add(new Employee(7, "Nathan Kreider", 2000, 500000.00, 5));
	    	employees.add(new Employee(8, "Durp Dur", 2000, 50000.00, 4));
	    	employees.add(new Employee(9, "Durp Hurp", 1821, 180912.00, 5));
	    	employees.add(new Employee(10, "Le Durp", 1921, 1920101.00, 2));
	    	employees.add(new Employee(11, "Nathan Kreider", 2000, 500000.00, 5));
	    	employees.add(new Employee(12, "What Durp", 1200, 500000.00, 1));
	    	employees.add(new Employee(13, "Oh No", 1800, 120000.00, 3));
	    	employees.add(new Employee(14, "Waddup Bruh", 2012, 180000.00, 1));
	    	employees.add(new Employee(15, "Jai Mason", 2007, 12000.00, 3));




Instead, you want something like
loadEmployees(dataFile);


where you've acquired the data file from your user with a FileChooser or some such means, and loadEmployees() is a method you write to read the contents of a properties file with this data in it.

I can't stress this enough: you absolutely need to know how to keep data and code separate. Properties files are the way to do this. It's a built-in class, so the documentation is in the standard library documentation. Read it and play with it, then try to make it work for you. This in itself will help you elevate your skills.

While you're doing that, you might also consider how you could use Properties files to keep the literal Strings out of the code. Imagine that you have a writer working for you who generates text, and they periodically decide to revise the stuff you present to the user. You'd prefer that they don't have to bug you to change "Employees who have been working here for 10 years or more." to "Old-timers" for that pithy, zingy feel they suddenly want, and you certainly don't ever want a tech writer to touch your code. You'd rather they could just edit a text file - well, Properties can make that work for you.

You want to review your use of try/catch blocks. A try block should enclose one potential failure point, ideally, and you should start figuring out how to recover from failure. Suppose you can't open an output file for writing - what can cause that, and how can your program work with the user to recover from that? (Intentionally vague: figure it out! Hint: exceptions are meant to be informative)

There are plenty of other things to think about, but you've got plenty to work on now.
Was This Post Helpful? 0
  • +
  • -

#11 Vampiricx3  Icon User is offline

  • D.I.C Head

Reputation: -2
  • View blog
  • Posts: 85
  • Joined: 01-December 11

Re: How can I improve this?

Posted 27 December 2011 - 08:59 AM

okay, I took baavgai's code, implemented it, except now there's no file output. urgh, fml.

View Postblackcompe, on 27 December 2011 - 08:12 AM, said:

In addition to breaking your code into intelligently-named single-responsibility methods as shown by baavgai, how about naming the class according to what is represents. Working tells me nothing about what the class does or represents. How about Company? Also, remember that identifiers (with the of exception boolean statuses) should be nouns or action verbs. E.g.

Company, Employee, addEmployee, writeEmployee, isInTraining (the exception).

If your method is longer than 15-20 lines it's probably too long. It's probably not cohesive. This isn't always true though, a sorting algorithm implementation is one example. To be cohesive is to stick to accomplishing a single thing. Yeah, you end with a lot more code, but readability is more important. Try to decompose it functionally. baavgai's code is clean, cohesive, and has minimal coupling (notice how he sent the Writer to method, instead of declaring it as a local variable).

private void writeAll(Writer out, Writer cOut) throws IOException


Your Employee class looks good. If you really want to become the best developer you can, read some books on refactoring and see how much better you code looks. I think there's a refactoring tutorial on this site.

Care to suggest some good tutorials/books on this? I'm willing to do some serious reading!

View Postjon.kiparsky, on 27 December 2011 - 08:23 AM, said:

View PostVampiricx3, on 27 December 2011 - 03:30 AM, said:

Can anyone tell me some shit to do to improve on my skills?


One obvious suggestion would be to look into the Properties class. This will allow you to externalize your data. You absolutely do not want this:

	    	employees.add(new Employee(1, "Taylah Jayne", 2000, 50000.00, 5));
	    	employees.add(new Employee(2, "Hurp Hurp", 1981, 1000000.00, 3));
	    	employees.add(new Employee(3, "Hurp Durp", 2011, 10000.00, 1));
	    	employees.add(new Employee(4, "4chan l00lz", 2004, 0.00, 2));
	    	employees.add(new Employee(5, "m00t harz", 2004, 200000.00, 3));
	    	employees.add(new Employee(6, "Derpina z0r", 2009, 15000.00, 1));
	    	employees.add(new Employee(7, "Nathan Kreider", 2000, 500000.00, 5));
	    	employees.add(new Employee(8, "Durp Dur", 2000, 50000.00, 4));
	    	employees.add(new Employee(9, "Durp Hurp", 1821, 180912.00, 5));
	    	employees.add(new Employee(10, "Le Durp", 1921, 1920101.00, 2));
	    	employees.add(new Employee(11, "Nathan Kreider", 2000, 500000.00, 5));
	    	employees.add(new Employee(12, "What Durp", 1200, 500000.00, 1));
	    	employees.add(new Employee(13, "Oh No", 1800, 120000.00, 3));
	    	employees.add(new Employee(14, "Waddup Bruh", 2012, 180000.00, 1));
	    	employees.add(new Employee(15, "Jai Mason", 2007, 12000.00, 3));




Instead, you want something like
loadEmployees(dataFile);


where you've acquired the data file from your user with a FileChooser or some such means, and loadEmployees() is a method you write to read the contents of a properties file with this data in it.

I can't stress this enough: you absolutely need to know how to keep data and code separate. Properties files are the way to do this. It's a built-in class, so the documentation is in the standard library documentation. Read it and play with it, then try to make it work for you. This in itself will help you elevate your skills.

While you're doing that, you might also consider how you could use Properties files to keep the literal Strings out of the code. Imagine that you have a writer working for you who generates text, and they periodically decide to revise the stuff you present to the user. You'd prefer that they don't have to bug you to change "Employees who have been working here for 10 years or more." to "Old-timers" for that pithy, zingy feel they suddenly want, and you certainly don't ever want a tech writer to touch your code. You'd rather they could just edit a text file - well, Properties can make that work for you.

You want to review your use of try/catch blocks. A try block should enclose one potential failure point, ideally, and you should start figuring out how to recover from failure. Suppose you can't open an output file for writing - what can cause that, and how can your program work with the user to recover from that? (Intentionally vague: figure it out! Hint: exceptions are meant to be informative)

There are plenty of other things to think about, but you've got plenty to work on now.

Do you just mean reading in information into an ArrayList from an external .txt file (serialization)? If so, care to share a tutorial on this? Thanks!
Was This Post Helpful? 0
  • +
  • -

#12 macosxnerd101  Icon User is online

  • Self-Trained Economist
  • member icon




Reputation: 10647
  • View blog
  • Posts: 39,540
  • Joined: 27-December 08

Re: How can I improve this?

Posted 27 December 2011 - 09:01 AM

Oracle has a tutorial on properties files.
Was This Post Helpful? 0
  • +
  • -

#13 jon.kiparsky  Icon User is online

  • Pancakes!
  • member icon


Reputation: 7875
  • View blog
  • Posts: 13,357
  • Joined: 19-March 11

Re: How can I improve this?

Posted 27 December 2011 - 09:07 AM

View Postmacosxnerd101, on 27 December 2011 - 11:01 AM, said:




Since you're looking to beef up your skills, start with the documentation. Nothing wrong with tutorials, of course, but reading the docs is a skill, just like writing code or more so. Don't miss a chance to practice it.
Was This Post Helpful? 1
  • +
  • -

#14 blackcompe  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 1155
  • View blog
  • Posts: 2,535
  • Joined: 05-May 05

Re: How can I improve this?

Posted 27 December 2011 - 11:33 AM

Quote

Care to suggest some good tutorials/books on this? I'm willing to do some serious reading!


Have a look at this forum post. I suggest Clean Code or Refactoring.
Was This Post Helpful? 0
  • +
  • -

Page 1 of 1