Skip to content

Sprint 7 solution in file manager #7

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

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

AestoDes
Copy link
Collaborator

No description provided.

Sla-als

This comment was marked as duplicate.

lirima11 added a commit to lirima11/java-kanban-1 that referenced this pull request Feb 10, 2025
* Delete src directory

* Delete .idea directory

* Delete lib directory
Comment on lines 8 to +41

public class Main {
public static void main(String[] args) {
System.out.println("Поехали!");
File file = new File("tasks.csv");
FileBackedTaskManager manager = new FileBackedTaskManager(file);

// Создаём задачи
Task task1 = new Task("Task 1", "Description 1", manager.generateId(), TaskStatus.NEW);
manager.createTask(task1);

Task task2 = new Task("Task 2", "Description 2", manager.generateId(), TaskStatus.NEW);
manager.createTask(task2);

// Создаём эпик
Epic epic1 = new Epic("Epic 1", "Epic Description", manager.generateId(), TaskStatus.NEW);
manager.createEpic(epic1);

// Добавляем подзадачи к эпику
Subtask subtask1 = new Subtask("Subtask 1", "Subtask Description", manager.generateId(), TaskStatus.NEW, epic1.getId());
manager.createSubtask(subtask1);

Subtask subtask2 = new Subtask("Subtask 2", "Subtask Description", manager.generateId(), TaskStatus.IN_PROGRESS, epic1.getId());
manager.createSubtask(subtask2);

// Выводим задачи перед сохранением
System.out.println("Tasks before loading: " + manager.getAllTasks());
System.out.println("Epics before loading: " + manager.getAllEpics());
System.out.println("Subtasks before loading: " + manager.getAllSubtasks());

// Загружаем задачи из файла
FileBackedTaskManager loadedManager = FileBackedTaskManager.loadFromFile(file);

// Выводим загруженные данные
System.out.println("Tasks after loading: " + loadedManager.getAllTasks());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В классе Main отсутствует метод main, из-за чего приложение не будет запускаться как самостоятельная программа. Рекомендую обернуть код в метод public static void main(String[] args).

Comment on lines +21 to +37
private void save() {
try (BufferedWriter writer = new BufferedWriter(new FileWriter(file))) {
writer.write("id,type,name,status,description,epic\n"); // Заголовок CSV

for (Task task : getAllTasks()) {
writer.write(toString(task) + "\n");
}
for (Epic epic : getAllEpics()) {
writer.write(toString(epic) + "\n");
}
for (Subtask subtask : getAllSubtasks()) {
writer.write(toString(subtask) + "\n");
}
} catch (IOException e) {
throw new ManagerSaveException("Error saving to file", e);
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Метод save() вызывается только при изменениях через create/update/delete, но не при загрузке из файла. Это может привести к рассинхронизации, если файл был изменён вручную. Рекомендую добавить валидацию или синхронизацию состояния после загрузки.

Comment on lines +52 to +68
private Task fromString(String value) {
String[] fields = value.split(",");
int id = Integer.parseInt(fields[0]);
String type = fields[1];
String title = fields[2];
TaskStatus status = TaskStatus.valueOf(fields[3]);
String description = fields[4];

if ("EPIC".equals(type)) {
return new Epic(title, description, id, status);
} else if ("SUBTASK".equals(type)) {
int epicId = Integer.parseInt(fields[5]);
return new Subtask(title, description, id, status, epicId);
} else {
return new Task(title, description, id, status);
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Метод fromString не обрабатывает возможные ошибки формата CSV (например, если строка повреждена или не хватает полей). Лучше добавить обработку ошибок с информативным исключением.

Comment on lines +71 to +104
public static FileBackedTaskManager loadFromFile(File file) {
FileBackedTaskManager manager = new FileBackedTaskManager(file);
Map<Integer, Subtask> subtasks = new HashMap<>();

try (BufferedReader reader = new BufferedReader(new FileReader(file))) {
reader.readLine(); // Пропускаем заголовок
String line;

while ((line = reader.readLine()) != null) {
Task task = manager.fromString(line);
if (task instanceof Epic) {
manager.createEpic((Epic) task);
} else if (task instanceof Subtask) {
subtasks.put(task.getId(), (Subtask) task);
} else {
manager.createTask(task);
}
}

// Восстановление связи подзадач с эпиками
for (Subtask subtask : subtasks.values()) {
manager.createSubtask(subtask);
Epic epic = manager.getEpic(subtask.getEpicId());
if (epic != null) {
epic.addSubtaskId(subtask.getId());
}
}

} catch (IOException e) {
throw new ManagerSaveException("Error loading from file", e);
}

return manager;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В методе loadFromFile при восстановлении связей между эпиками и подзадачами возможна ситуация, когда эпик не найден (например, если файл повреждён). Лучше явно логировать такие случаи или выбрасывать исключение.

Comment on lines +12 to +141
}
return task;
}

@Override
public void createEpic(Epic epic) {
epics.put(epic.getId(), epic);
}

@Override
public Epic getEpic(int id) {
Epic epic = epics.get(id);
if (epic != null) {
historyManager.add(epic);
}
return epic;
}

@Override
public void createSubtask(Subtask subtask) {
subtasks.put(subtask.getId(), subtask);
Epic epic = epics.get(subtask.getEpicId());
if (epic != null) {
epic.addSubtaskId(subtask.getId());
}
}

@Override
public Subtask getSubtask(int id) {
Subtask subtask = subtasks.get(id);
if (subtask != null) {
historyManager.add(subtask);
}
return subtask;
}

@Override
public List<Subtask> getSubtasksOfEpic(int epicId) {
List<Subtask> epicSubtasks = new ArrayList<>();
Epic epic = epics.get(epicId);
if (epic != null) {
for (Integer subtaskId : epic.getSubtaskIds()) {
if (subtasks.containsKey(subtaskId)) {
epicSubtasks.add(subtasks.get(subtaskId));
}
}
}
return epicSubtasks;
}

@Override
public void deleteTaskById(int id) {
tasks.remove(id);
historyManager.remove(id);
}

@Override
public void deleteSubtaskById(int id) {
Subtask subtask = subtasks.remove(id);
if (subtask != null) {
Epic epic = epics.get(subtask.getEpicId());
if (epic != null) {
epic.getSubtaskIds().remove((Integer) id);
}
}
historyManager.remove(id);
}

@Override
public void deleteEpicById(int id) {
Epic epic = epics.remove(id);
if (epic != null) {
for (Integer subtaskId : epic.getSubtaskIds()) {
subtasks.remove(subtaskId);
historyManager.remove(subtaskId);
}
}
historyManager.remove(id);
}

@Override
public List<Task> getAllTasks() {
return new ArrayList<>(tasks.values());
}

@Override
public List<Epic> getAllEpics() {
return new ArrayList<>(epics.values());
}

@Override
public List<Subtask> getAllSubtasks() {
return new ArrayList<>(subtasks.values());
}

@Override
public List<Task> getHistory() {
return historyManager.getHistory();
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В InMemoryTaskManager не реализовано автоматическое обновление статуса эпика при изменении статусов подзадач. Обычно статус эпика должен вычисляться на основе статусов его подзадач (например, если все подзадачи DONE — эпик DONE, если есть IN_PROGRESS — эпик IN_PROGRESS и т.д.). Рекомендую добавить соответствующую логику.

Comment on lines +8 to +41

public interface TaskManager {
int generateId();

void createTask(Task task);

void createEpic(Epic epic);

void createSubtask(Subtask subtask);

Task getTask(int id);

Epic getEpic(int id);

Subtask getSubtask(int id);

void updateTask(Task task);

void deleteTaskById(int id);

void deleteEpicById(int id);

void deleteSubtaskById(int id);

List<Task> getAllTasks();

List<Epic> getAllEpics();

List<Subtask> getAllSubtasks();

List<Subtask> getSubtasksOfEpic(int epicId);

List<Task> getHistory();
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В интерфейсе TaskManager отсутствуют методы для обновления и удаления подзадач и эпиков (есть только updateTask). Для полноты интерфейса стоит добавить updateEpic и updateSubtask.

Comment on lines +1 to +25
import org.junit.jupiter.api.Test;
import tasktracker.manager.FileBackedTaskManager;

import java.io.File;
import java.io.IOException;

import static org.junit.jupiter.api.Assertions.assertTrue;

public class FileBackedTaskManagerTest {

@Test
public void shouldLoadFromEmptyFile() throws IOException {
// Создаём временный пустой файл
File file = File.createTempFile("empty_tasks", ".csv");
file.deleteOnExit(); // Удаляется после завершения теста

// Загружаем менеджер из пустого файла
FileBackedTaskManager loadedManager = FileBackedTaskManager.loadFromFile(file);

// Проверяем, что данные пустые
assertTrue(loadedManager.getAllTasks().isEmpty(), "Tasks list should be empty");
assertTrue(loadedManager.getAllEpics().isEmpty(), "Epics list should be empty");
assertTrue(loadedManager.getAllSubtasks().isEmpty(), "Subtasks list should be empty");
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тесты покрывают базовые сценарии, но не проверяют граничные случаи (например, удаление несуществующей задачи, корректность истории при превышении лимита, восстановление связей между эпиками и подзадачами после загрузки из файла). Рекомендую добавить такие тесты для повышения надёжности.

Comment on lines 8 to +41

public class Main {
public static void main(String[] args) {
System.out.println("Поехали!");
File file = new File("tasks.csv");
FileBackedTaskManager manager = new FileBackedTaskManager(file);

// Создаём задачи
Task task1 = new Task("Task 1", "Description 1", manager.generateId(), TaskStatus.NEW);
manager.createTask(task1);

Task task2 = new Task("Task 2", "Description 2", manager.generateId(), TaskStatus.NEW);
manager.createTask(task2);

// Создаём эпик
Epic epic1 = new Epic("Epic 1", "Epic Description", manager.generateId(), TaskStatus.NEW);
manager.createEpic(epic1);

// Добавляем подзадачи к эпику
Subtask subtask1 = new Subtask("Subtask 1", "Subtask Description", manager.generateId(), TaskStatus.NEW, epic1.getId());
manager.createSubtask(subtask1);

Subtask subtask2 = new Subtask("Subtask 2", "Subtask Description", manager.generateId(), TaskStatus.IN_PROGRESS, epic1.getId());
manager.createSubtask(subtask2);

// Выводим задачи перед сохранением
System.out.println("Tasks before loading: " + manager.getAllTasks());
System.out.println("Epics before loading: " + manager.getAllEpics());
System.out.println("Subtasks before loading: " + manager.getAllSubtasks());

// Загружаем задачи из файла
FileBackedTaskManager loadedManager = FileBackedTaskManager.loadFromFile(file);

// Выводим загруженные данные
System.out.println("Tasks after loading: " + loadedManager.getAllTasks());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В классе Main отсутствует метод main, из-за чего этот класс не будет запускаться как точка входа в приложение. Рекомендую обернуть код в метод public static void main(String[] args), чтобы обеспечить корректный запуск программы.

Comment on lines +21 to +37
private void save() {
try (BufferedWriter writer = new BufferedWriter(new FileWriter(file))) {
writer.write("id,type,name,status,description,epic\n"); // Заголовок CSV

for (Task task : getAllTasks()) {
writer.write(toString(task) + "\n");
}
for (Epic epic : getAllEpics()) {
writer.write(toString(epic) + "\n");
}
for (Subtask subtask : getAllSubtasks()) {
writer.write(toString(subtask) + "\n");
}
} catch (IOException e) {
throw new ManagerSaveException("Error saving to file", e);
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Метод save() вызывается только при изменениях через create/update/delete, но не при загрузке из файла. Это хорошо, однако стоит добавить тесты на случай, если файл повреждён или содержит некорректные данные. Также, если файл пустой (только заголовок), fromString может выбросить исключение. Рекомендую добавить проверку на пустые строки и корректность количества полей при парсинге CSV.

Comment on lines +52 to +68
private Task fromString(String value) {
String[] fields = value.split(",");
int id = Integer.parseInt(fields[0]);
String type = fields[1];
String title = fields[2];
TaskStatus status = TaskStatus.valueOf(fields[3]);
String description = fields[4];

if ("EPIC".equals(type)) {
return new Epic(title, description, id, status);
} else if ("SUBTASK".equals(type)) {
int epicId = Integer.parseInt(fields[5]);
return new Subtask(title, description, id, status, epicId);
} else {
return new Task(title, description, id, status);
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Метод fromString не проверяет количество полей в строке, что может привести к ArrayIndexOutOfBoundsException при некорректных данных. Рекомендую добавить валидацию длины массива fields и обработку ошибок парсинга.

Comment on lines +71 to +104
public static FileBackedTaskManager loadFromFile(File file) {
FileBackedTaskManager manager = new FileBackedTaskManager(file);
Map<Integer, Subtask> subtasks = new HashMap<>();

try (BufferedReader reader = new BufferedReader(new FileReader(file))) {
reader.readLine(); // Пропускаем заголовок
String line;

while ((line = reader.readLine()) != null) {
Task task = manager.fromString(line);
if (task instanceof Epic) {
manager.createEpic((Epic) task);
} else if (task instanceof Subtask) {
subtasks.put(task.getId(), (Subtask) task);
} else {
manager.createTask(task);
}
}

// Восстановление связи подзадач с эпиками
for (Subtask subtask : subtasks.values()) {
manager.createSubtask(subtask);
Epic epic = manager.getEpic(subtask.getEpicId());
if (epic != null) {
epic.addSubtaskId(subtask.getId());
}
}

} catch (IOException e) {
throw new ManagerSaveException("Error loading from file", e);
}

return manager;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В методе loadFromFile связь между эпиками и подзадачами восстанавливается вручную, что может привести к дублированию подзадач в списке эпика, если createSubtask уже добавляет id в epic. Лучше явно контролировать этот момент или добавить комментарий, почему делается двойное добавление.

Comment on lines +12 to +141
}
return task;
}

@Override
public void createEpic(Epic epic) {
epics.put(epic.getId(), epic);
}

@Override
public Epic getEpic(int id) {
Epic epic = epics.get(id);
if (epic != null) {
historyManager.add(epic);
}
return epic;
}

@Override
public void createSubtask(Subtask subtask) {
subtasks.put(subtask.getId(), subtask);
Epic epic = epics.get(subtask.getEpicId());
if (epic != null) {
epic.addSubtaskId(subtask.getId());
}
}

@Override
public Subtask getSubtask(int id) {
Subtask subtask = subtasks.get(id);
if (subtask != null) {
historyManager.add(subtask);
}
return subtask;
}

@Override
public List<Subtask> getSubtasksOfEpic(int epicId) {
List<Subtask> epicSubtasks = new ArrayList<>();
Epic epic = epics.get(epicId);
if (epic != null) {
for (Integer subtaskId : epic.getSubtaskIds()) {
if (subtasks.containsKey(subtaskId)) {
epicSubtasks.add(subtasks.get(subtaskId));
}
}
}
return epicSubtasks;
}

@Override
public void deleteTaskById(int id) {
tasks.remove(id);
historyManager.remove(id);
}

@Override
public void deleteSubtaskById(int id) {
Subtask subtask = subtasks.remove(id);
if (subtask != null) {
Epic epic = epics.get(subtask.getEpicId());
if (epic != null) {
epic.getSubtaskIds().remove((Integer) id);
}
}
historyManager.remove(id);
}

@Override
public void deleteEpicById(int id) {
Epic epic = epics.remove(id);
if (epic != null) {
for (Integer subtaskId : epic.getSubtaskIds()) {
subtasks.remove(subtaskId);
historyManager.remove(subtaskId);
}
}
historyManager.remove(id);
}

@Override
public List<Task> getAllTasks() {
return new ArrayList<>(tasks.values());
}

@Override
public List<Epic> getAllEpics() {
return new ArrayList<>(epics.values());
}

@Override
public List<Subtask> getAllSubtasks() {
return new ArrayList<>(subtasks.values());
}

@Override
public List<Task> getHistory() {
return historyManager.getHistory();
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В InMemoryTaskManager не реализована логика автоматического обновления статуса эпика в зависимости от статусов его подзадач. Обычно статус эпика вычисляется на основе статусов всех его подзадач (например, если все подзадачи DONE — эпик DONE, если все NEW — эпик NEW, иначе IN_PROGRESS). Рекомендую реализовать этот механизм для корректной работы с эпиками.

Comment on lines +8 to +41

public interface TaskManager {
int generateId();

void createTask(Task task);

void createEpic(Epic epic);

void createSubtask(Subtask subtask);

Task getTask(int id);

Epic getEpic(int id);

Subtask getSubtask(int id);

void updateTask(Task task);

void deleteTaskById(int id);

void deleteEpicById(int id);

void deleteSubtaskById(int id);

List<Task> getAllTasks();

List<Epic> getAllEpics();

List<Subtask> getAllSubtasks();

List<Subtask> getSubtasksOfEpic(int epicId);

List<Task> getHistory();
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В интерфейсе TaskManager методы updateEpic и updateSubtask отсутствуют, хотя для симметрии с updateTask их стоит добавить. Это повысит расширяемость и удобство использования интерфейса.

Comment on lines +1 to +25
import org.junit.jupiter.api.Test;
import tasktracker.manager.FileBackedTaskManager;

import java.io.File;
import java.io.IOException;

import static org.junit.jupiter.api.Assertions.assertTrue;

public class FileBackedTaskManagerTest {

@Test
public void shouldLoadFromEmptyFile() throws IOException {
// Создаём временный пустой файл
File file = File.createTempFile("empty_tasks", ".csv");
file.deleteOnExit(); // Удаляется после завершения теста

// Загружаем менеджер из пустого файла
FileBackedTaskManager loadedManager = FileBackedTaskManager.loadFromFile(file);

// Проверяем, что данные пустые
assertTrue(loadedManager.getAllTasks().isEmpty(), "Tasks list should be empty");
assertTrue(loadedManager.getAllEpics().isEmpty(), "Epics list should be empty");
assertTrue(loadedManager.getAllSubtasks().isEmpty(), "Subtasks list should be empty");
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants