-
Notifications
You must be signed in to change notification settings - Fork 28
Issue185 v2 #10
Issue185 v2 #10
Changes from 5 commits
aad8b8f
5a37d8e
974315e
9cde669
7d71000
18d8785
b14f4d1
a5cff4b
5cedbdd
a275c59
e877178
911d681
dac99b7
e2aa549
cc2f13b
21a048c
222a065
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,6 +50,7 @@ | |
import esmska.data.Config; | ||
import esmska.data.Gateways; | ||
import esmska.data.History; | ||
import esmska.data.History.Record; | ||
import esmska.data.Icons; | ||
import esmska.data.Log; | ||
import esmska.data.Queue; | ||
|
@@ -710,15 +711,35 @@ private boolean saveAll() { | |
} | ||
} | ||
|
||
/** Saves history of sent sms */ | ||
/** | ||
* Saves history of sent sms | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's preferable if you don't reformat lines that don't need to be reformatted. |
||
*/ | ||
private void createHistory(SMS sms) { | ||
History.Record record = new History.Record(sms.getNumber(), sms.getText(), | ||
|
||
boolean match = false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why exactly do you need this variable? You can easily distinguish whether you've found something based on |
||
List<Record> records = history.getRecords(); | ||
for(int i = records.size()-1; i >= 0; i--){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, come on! What about:
Isn't that more readable and less error prone than counting indices? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, please wrap the whole searching section in
If it is |
||
Record r = records.get(i); | ||
if (sms.getId() != null) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we wrap the whole search section, this line can go away. |
||
if (sms.getId().equals(r.getSmsId())) { | ||
r.setText(r.getText() + sms.getText()); | ||
r.setDate(null); | ||
match = true; | ||
break; | ||
} | ||
} | ||
} | ||
if(!match){ | ||
Record record = new Record(sms.getNumber(), sms.getText(), | ||
sms.getGateway(), sms.getName(), sms.getSenderNumber(), | ||
sms.getSenderName(), null); | ||
history.addRecord(record); | ||
sms.getSenderName(), null, sms.getId()); | ||
history.addRecord(record); | ||
} | ||
} | ||
|
||
/** save program configuration | ||
|
||
/** | ||
* save program configuration | ||
* | ||
* @return true if saved ok; false otherwise | ||
*/ | ||
private boolean saveConfig() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -125,11 +125,12 @@ public static ArrayList<History.Record> importHistory(File file) throws Exceptio | |
String text = reader.get(4); | ||
String senderName = reader.get(5); | ||
String senderNumber = reader.get(6); | ||
String id = reader.get(7); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This works, if you have an empty history. But people already have some history, and it doesn't contain "id" yet. So you need to convert their history into the new format first. Please have a look at |
||
|
||
Date date = df.parse(dateString); | ||
|
||
History.Record record = new History.Record(number, text, gateway, | ||
name, senderNumber, senderName, date); | ||
name, senderNumber, senderName, date, id); | ||
history.add(record); | ||
} catch (Exception e) { | ||
logger.severe("Invalid history record: " + reader.getRawRecord()); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please better document what happens if you provide a real value and if you provide
null
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still applies. I imagine something like: