Skip to content

Fix Create & Update button #4

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

Open
moketa99 wants to merge 1 commit into
base: complete
Choose a base branch
from
Open

Fix Create & Update button #4

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 53 additions & 21 deletions src/main/java/edu/mills/cs180a/wordui/FXMLController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import edu.mills.cs180a.wordui.model.SampleData;
import edu.mills.cs180a.wordui.model.WordRecord;
import edu.mills.cs180a.wordui.model.WordRecord.SortOrder;
import javafx.beans.binding.Bindings;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.StringProperty;
Expand All @@ -19,15 +18,12 @@
import javafx.fxml.Initializable;
import javafx.scene.control.Button;
import javafx.scene.control.ChoiceBox;
import javafx.scene.control.Label;
import javafx.scene.control.ListView;
import javafx.scene.control.TextArea;
import javafx.scene.control.TextField;
import javafx.scene.input.KeyEvent;

public class FXMLController implements Initializable {
@FXML
private Label recordCountLabel;
@FXML
private TextField wordTextField;
@FXML
Expand All @@ -50,9 +46,12 @@ public class FXMLController implements Initializable {
private WordRecord selectedWordRecord;
private final BooleanProperty modifiedProperty = new SimpleBooleanProperty(false);
private final BooleanProperty freqValidProperty = new SimpleBooleanProperty(false);
private final BooleanProperty wordExistPropertyCreate = new SimpleBooleanProperty(false);
private final BooleanProperty wordExistPropertyUpdate = new SimpleBooleanProperty(false);

@ellenspertus ellenspertus Nov 28, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you explain to me why both of these variables are needed?

@moketa99 moketa99 Dec 6, 2020 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

If a word is updated to a word that is already in the list, then the same word is in the list twice.
The program controls that the same word is not displayed in the list.

It also controls that the update button is disabled if a user tries to update a word and stop it in the middle, then come back to the original word.

For example,
If a user changes "buffalo" to "buffalo bills", the update button is enabled. But if the user changes "buffalo bills" to "buffalo", the program has to disable the update button. However, if the user changes the definition, the program has to enable the update button.

I use them to control like that.

"Complete the TODO in FXMLController.configureButtons(). "
Since I read the above instructions, I understood that all buttons on the display would be modified to control them correctly.

@ellenspertus ellenspertus Dec 7, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You did more work than requested. It read:

// TODO: Disable the Create button if an existing entry is selected or any
// field is empty or invalid.

@ellenspertus ellenspertus Dec 7, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My reading of the code suggests that the two variables always have exactly the same value. Could you please show me any lines where one is set to a different value than the other?

@moketa99 moketa99 Dec 7, 2020 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think they are not always the same and update and create buttons need different input checks for an existing entry.
The update does a unique check without itself.
The create always does a unique check.

I understand "if an existing entry is selected" means
When the user selects an existing entry, disable the Create button.
When the user updates a word, enable the Create button.
When the user updates a word to an existing entry (the same situation as an existing entry is selected), disable the Create button
Do I misunderstand instruction?

private ChangeListener<WordRecord> wordRecordChangeListener = new WordRecordChangeListener();
private ChangeListener<WordRecord.SortOrder> sortOrderChangeListener =
new SortOrderChangeListener();
private WordRecord.SortOrder currentSort;

// Called when the user selects or unselects a WordRecord.
private class WordRecordChangeListener implements ChangeListener<WordRecord> {
Expand All @@ -63,14 +62,19 @@ public void changed(ObservableValue<? extends WordRecord> observable, WordRecord
System.out.println("Selected item: " + newValue);
selectedWordRecord = newValue;
modifiedProperty.set(false);

if (newValue != null) {
wordTextField.setText(selectedWordRecord.getWord());
frequencyTextField.setText(Integer.toString(selectedWordRecord.getFrequency()));
wordExistPropertyCreate.set(isExistWordCreate(wordTextField.getText()));
wordExistPropertyUpdate.set(isExistWordCreate(wordTextField.getText()));
freqValidProperty.set(isValidFrequency(frequencyTextField.textProperty()));
definitionTextArea.setText(selectedWordRecord.getDefinition());
} else {
wordTextField.setText("");
frequencyTextField.setText("");
wordExistPropertyCreate.set(false);
wordExistPropertyUpdate.set(false);
freqValidProperty.set(false);
definitionTextArea.setText("");
}
Expand All @@ -89,36 +93,32 @@ private void setSortOrder(WordRecord.SortOrder newOrder) {
SortedList<WordRecord> sortedList = new SortedList<>(wordRecordList);
sortedList.setComparator(newOrder.getComparator());
listView.setItems(sortedList);
currentSort = newOrder;
}

@Override
public void initialize(URL url, ResourceBundle rb) {
addListeners();
setupListView();
configureButtons();
populateChoiceBox();
}

private void setupListView() {
// Initialize the list.
SampleData.fillSampleData(wordRecordList);

configureButtons();

// Sort list alphabetically.
setSortOrder(WordRecord.SortOrder.ALPHABETICALLY_FORWARD);

// Set up the record count. This must occur after listView is populated.
recordCountLabel.textProperty().bind(Bindings.size(listView.getItems()).asString());
populateChoiceBox();
addListeners();

// Pre-select the first item.
listView.getSelectionModel().selectFirst();
}

private void populateChoiceBox() {
sortChoiceBox.setItems(FXCollections.observableArrayList(
WordRecord.SortOrder.values()));
sortChoiceBox.setItems(FXCollections.observableArrayList(WordRecord.SortOrder.values()));
sortChoiceBox.setValue(WordRecord.SortOrder.ALPHABETICALLY_FORWARD);
}

//@formatter:off
private void configureButtons() {
// Disable the Remove button if nothing is selected in the ListView control.
removeButton.disableProperty()
Expand All @@ -129,12 +129,43 @@ private void configureButtons() {
updateButton.disableProperty()
.bind(listView.getSelectionModel().selectedItemProperty().isNull()
.or(modifiedProperty.not())
.or(wordExistPropertyUpdate.not())
.or(freqValidProperty.not())
.or(wordTextField.textProperty().isEmpty())
.or(definitionTextArea.textProperty().isEmpty()));

// TODO: Disable the Create button if an existing entry is selected or any
// Disable the Create button if an existing entry is selected or any
// field is empty or invalid.
createButton.disableProperty()
.bind(listView.getSelectionModel().selectedItemProperty().isNull()
.or(modifiedProperty.not())
.or(wordExistPropertyCreate.not())
.or(freqValidProperty.not())
.or(wordTextField.textProperty().isEmpty())
.or(definitionTextArea.textProperty().isEmpty()));
}
//@formatter:on

// Return false if the word exists when create.
private boolean isExistWordCreate(String s) {

@ellenspertus ellenspertus Nov 28, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very good.

@ellenspertus ellenspertus Nov 28, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A better name might be doesWordExist or wordExists.

@moketa99 moketa99 Dec 6, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you for your comment. I will fix it.

for (WordRecord list : wordRecordList) {
if (list.getWord().equals(s))
return false;
}
return true;
}

// Return false if the word exists without itself when update.
private boolean doesWordExistUpdate(String s) {
if (s.equals(selectedWordRecord.getWord())) {
return true;
} else {
for (WordRecord list : wordRecordList) {
if (list.getWord().equals(s))
return false;
}
}
return true;
}

// A frequency is valid if it is an integer and is at least 0.
Expand All @@ -157,16 +188,16 @@ private void addListeners() {
private void handleKeyAction(KeyEvent keyEvent) {
modifiedProperty.set(true);
freqValidProperty.set(isValidFrequency(frequencyTextField.textProperty()));
wordExistPropertyCreate.set(isExistWordCreate(wordTextField.getText()));
wordExistPropertyUpdate.set(doesWordExistUpdate(wordTextField.getText()));
}

@FXML
private void createButtonAction(ActionEvent actionEvent) {
System.out.println("Create");
WordRecord wordRecord =
new WordRecord(
wordTextField.getText(),
Integer.parseInt(frequencyTextField.getText()),
definitionTextArea.getText());

WordRecord wordRecord = new WordRecord(wordTextField.getText(),
Integer.parseInt(frequencyTextField.getText()), definitionTextArea.getText());
wordRecordList.add(wordRecord);
listView.getSelectionModel().select(wordRecord); // select the new item
}
Expand All @@ -188,5 +219,6 @@ private void updateButtonAction(ActionEvent actionEvent) {
entry.setDefinition(definitionTextArea.getText());
listView.getSelectionModel().selectedItemProperty().addListener(wordRecordChangeListener);
modifiedProperty.set(false);
setSortOrder(currentSort);
}
}