LINUX.ORG.RU

[Java] упростить/укоротить код


0

1

Вчера и позавчера писал программу, которая читает файл и потом рисует диаграмму, в итоге у меня ощущение, что это как-то всё неакуратно и громоздко, то, что я написал. «Сложность» заключалась в том, что из текстового файла формата:

1999
I
Student_Name0 8989 87
Student_Name1 6855 34

-2000
=IMM
Student_Name3 6767 77
....и т.д.

нужно было считать всё, но использовать только проценты (которые стоят только в полях с именами студентов - поседние две цифры), и потом отсортировать их по заданному пользователем числу колонок (т.е. какое количество студентов имеет от 0 до 9%, 10-19% - это если пользователь задал 10 колонок).

Интерфейс самой диаграммы был написан за меня, поэтому моя программа только сортирует колонки и рисует диаграмму. Соб-но сам код:


package statistics;

import java.text.DecimalFormat;
import java.util.Scanner;
import java.lang.String;
import ui.StaafDiagramUserInterface; // интрфейс диаграммы
import ui.UserInterfaceFactory;
import ui.UIHulpMethodes;

class Statistics {
	
	UIHulpMethodes hlp;
	StaafDiagramUserInterface sd;
	
	int aantalStaven, lineLength, numberOfColumns, indexOfColumn;
	
	Statistics() {
		
		Scanner rangeOfDiagram = new Scanner(System.in); 
		System.out.printf("Количество колонок: ");
		numberOfColumns = rangeOfDiagram.nextInt(); 
		aantalStaven = numberOfColumns; // aantalStaven - число колонок - заданная в интерфейсе диаграммы переменная
		
		sd = UserInterfaceFactory.geefStaafDiagramUI(aantalStaven);
		UIHulpMethodes.vraagGebruikerOmInvoer(); // спросить пользователя файл
	}
	
	void parseLine(String readLine) { 
		Scanner readNextScanner = new Scanner(readLine);
		 												
		for (int i = 0; i < readLine.length(); i++) { 
									//read length of line 
			lineLength = i;	
		}
		
		if (lineLength < 7) {
			String yearsAndStudies = readNextScanner.next();
			Scanner readYearsAndStudiesScanner = new Scanner(yearsAndStudies);
			
			readYearsAndStudies(readYearsAndStudiesScanner);
			
		} 
		else { 	// if line is longer than 7 read it and divide names and game results
			readNextScanner.useDelimiter("\t");
					
			String studentNames = readNextScanner.next();
		
			String gameResults = readNextScanner.next();
			Scanner gameResultsScanner = new Scanner(gameResults);
					
			sortPercentagesIntoColumns(gameResultsScanner); // then call function to sort percentages;
		}
	}
	
	
	void readYearsAndStudies(Scanner readYearsAndStudiesScanner) {
		
		String readYearsAndStudies = readYearsAndStudiesScanner.next();
		
		if (readYearsAndStudies.matches("-")) {
			String years = readYearsAndStudiesScanner.next();
		} 
		else if (readYearsAndStudies.matches("I")) {
			String study = "I";
		} 
		else if (readYearsAndStudies.matches("=")) {
			String study = readYearsAndStudiesScanner.next();
		}
	}
	
	
	void sortPercentagesIntoColumns(Scanner gameResultsScanner) {
		
		String averageWaittime = gameResultsScanner.next(); 	 
		String percentageOfStones = gameResultsScanner.next();
		
		int currentPercentage = Integer.parseInt(percentageOfStones); // parse percentages to type int;
		// here are defined two variables which calculate "size" of columns in diagram by division of 100;  
		double step = 100.0 / numberOfColumns;  
		double adder = 100.0 / numberOfColumns;  
		indexOfColumn = 0;					
								// this part of function "walks" through all columns
								// in order to decide which index of column to increase; 
		while (step < currentPercentage) {	// so, while step is lower than a given percentage 
			step += adder;					// it increases variable "step" by "adder" and "steps"
			indexOfColumn++;				// to the next column;
		}
		
		if (indexOfColumn == numberOfColumns) { // if loop stepped over the last column
			indexOfColumn--;					// add index to this last column;
		}
		
		sd.verhoogStaaf(indexOfColumn); // добавить один к колонке
	}

	void start() {
		Scanner in = new Scanner(System.in); 
		
		while(in.hasNext()) {
			String readLine = in.nextLine();
			
			parseLine(readLine);
		}
		
		// variables to create an array of strings (names of columns);
		double adder = 100.0 / numberOfColumns;
		double from = 0;                     // every column has a name of "from" and "to" in percentages;
		double to = 100.0 / numberOfColumns - 1; // -1 here because names should be in format 0-9% , 10-19%, etc..
		
		DecimalFormat df = new DecimalFormat("#.#"); 
		
		for (indexOfColumn = 0; indexOfColumn < numberOfColumns; indexOfColumn++) {
			if (indexOfColumn == numberOfColumns - 1) { 
			to = 100; // if it's on the last column, assign "to" to 100%;
			} 

			sd.geefStaafNaam(indexOfColumn, df.format(from) + "-" + df.format(to) + "%");
			sd.zetLaatNamenZien(true);
			sd.toon();
			from += adder; // use "adder" "to walk" through array;
			to += adder; 
		}
	}

public static void main(String[] argv) {
	new Statistics().start();
	}
}

выглядит в итоге это всё вот так:

http://www.freeimagehosting.net/uploads/c27b76a856.png

вопрос - какие части можно упростить/сделать иначе/укоротить и т.д. ..?


А зачем было разделять конструктор и start()?

aantalStaven = numberOfColumns;
//...
String namesOfColumns[] = new String[numberOfColumns];
//...
for (indexOfColumn = 0; indexOfColumn < namesOfColumns.length; indexOfColumn++)
//...
for (indexOfColumn = 0; indexOfColumn < aantalStaven; indexOfColumn++)

А через неделю кто-нибудь догадается, что namesOfColumns.length == aantalStaven?
И зачем здесь два цикла, когда можно обойтись одним, кажется?

zhuravlik ★★★★ ()
Ответ на: комментарий от Sonsee

Вместо
String namesOfColumns[] = new String[numberOfColumns];


for (indexOfColumn = 0; indexOfColumn < namesOfColumns.length; indexOfColumn++) {

if (indexOfColumn == numberOfColumns - 1) {
to = 100; // if it's on the last column, assign «to» to 100%;
}

namesOfColumns[indexOfColumn] = df.format(from) + "-" + df.format(to) + «%»;
from += adder; // use «adder» «to walk» through array;
to += adder;
}

for (indexOfColumn = 0; indexOfColumn < aantalStaven; indexOfColumn++) { // draw diagram;

sd.geefStaafNaam(indexOfColumn, namesOfColumns[indexOfColumn]);
sd.zetLaatNamenZien(true);
sd.toon();
}

Пишем так:

for (indexOfColumn = 0; indexOfColumn < numberOfColumns; indexOfColumn++) {
if (indexOfColumn == numberOfColumns - 1) {
to = 100; // if it's on the last column, assign «to» to 100%;
}

sd.geefStaafNaam(indexOfColumn, df.format(from) + "-" + df.format(to) + «%»);
sd.zetLaatNamenZien(true);
sd.toon();
from += adder; // use «adder» «to walk» through array;
to += adder;
}

Профит:
минус массив
минус цикл

Потому что вроде бы этот массив больше нигде не используется, а второй цикл никак не пользуется результатами первого и оба цикла пробегают один набор значений. Поправьте, если не так.

zhuravlik ★★★★ ()
Ответ на: комментарий от zhuravlik

да я учу Java вторую неделю, и это первый мой язык ))), поэтому такие вопросы ;)

есть, может, ещё замечания? - меня например смущает отсутствие альтернативы line.length() (BufferedReader) для класса Scanner, или там, может, ещё как-то можно?

Sonsee ()
Ответ на: комментарий от Sonsee

А =)

Есть такие предложения:

1) В коде много неиспользуемых переменных. Можно попробовать их убрать.

Код методов, сократившихся в результате до нескольких строк и используемых только один раз, можно вклеить в вызывающий метод.

2) Подумайте, как придется перекраивать код, если понадобится выдать статистику за один заданный год, за несколько заданных лет, выдать статистику по студентам с id от k до l, или по алфавиту для всех студентов от А до Е, например.

Это поможет понять, что еще можно поменять в коде, чтобы можно было его быстро адаптировать к новым требованиям. (Этот совет может противоречить п.1 =)

zhuravlik ★★★★ ()
Ответ на: комментарий от Sonsee

предлагаю вам блестнуть знаниями, и показать пример )) 

Файлик длинноватый ~ 500 строк, парсится SQL. Но могу сказать, что без применения регэкспов этот файлик был в два раза толще, абсолютно не читабельный и не поддерживаемый.

Вот сниппет оттуда, он вырезает из файла определение вьюшки:


import java.util.regex.Matcher;
import java.util.regex.Pattern;

        		//...
        		String sRes = "";
        		//...
        		// get first VIEW definition
        		Pattern pattern = Pattern.compile(
        				"create\\s+or\\s+replace\\s+view(\\s+.*)*;", 
        				Pattern.CASE_INSENSITIVE);
        		Matcher matcher = pattern.matcher(_SQL);
        		if(matcher.find())	
        			sRes = matcher.group();

_SQL - входной параметр типа String (строка в которую считан и предварительно обработан SQL файл) Ну а дальше - гуглим :) Ещё скажу, что если вы парсите свои велосипедные форматы, то регэкспы - самый короткий путь к успеху.

gandjubas ()
Ответ на: комментарий от gandjubas

И ещё, это работает, но лучше писать вот так:


"create\\s+or\\s+replace\\s+view(\\s+[^;]+)*;"

или типа того, потому что регэкспы, бывает и виснут, если криво написаны (собственно я писал это по ходу изучения, поэтому).

gandjubas ()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.