Minor updates to BibEntry #1309
Conversation
koppor
added
the
status: ready-for-review
Current coverage is 27.39%@@ master #1309 diff @@
==========================================
Files 694 694
Lines 46428 46435 +7
Methods 0 0
Messages 0 0
Branches 7688 7690 +2
==========================================
Hits 12720 12720
- Misses 32611 32618 +7
Partials 1097 1097
|
| * Constructs a new BibEntry with the given ID and given type | ||
| * | ||
| * @param type The type to set. May be null or empty. In that case, DEFAULT_TYPE is used. | ||
| */ |
There was a problem hiding this comment.
👍 for creating JavaDoc - but please add the second parameter, too 😉
|
LGTM - apart from the small JavaDoc issues |
| @@ -44,7 +44,7 @@ public void init() throws IOException, SaveException { | |||
| Random randomizer = new Random(); | |||
| for (int i = 0; i < 1000; i++) { | |||
| BibEntry entry = new BibEntry(); | |||
| entry.setCiteKey("id" + i); | |||
| entry.setBibTeXKey("id" + i); | |||
There was a problem hiding this comment.
I think set/getKey would work too and might be preferable.
There was a problem hiding this comment.
I wanted to be explicit to avoid confusion, because someone might think that a key is an id. And keys are used in SQL-based databases, too. "primary key", "foreign key", etc.
There was a problem hiding this comment.
Why not introduce classes like BibTeXKey vs. InternalJabRefKey to distinguish them? Both can wrap a String inside.
There was a problem hiding this comment.
Are you talking about the general naming of the method or about the concrete usage of setBibTeXKey(...) in this class? 😕
I think the proposed naming and reasoning of @koppor is fine. ID for internal usage - "BibTeXKey" used as in the UI...
Using additional classes for this seems to be an overkill for me.
There was a problem hiding this comment.
Hm, what is your problem with getCiteKey or getKey and getID? The description makes it clear what are the differences. Also the entry is a BibEntry, so so why clutter the method name with BibTeX again? It's not a big deal, but I think a crisper method name is nicer here.
There was a problem hiding this comment.
According to WikiPedia's BibTeX article, there is a bibtex field key, which is ...
A hidden field used for specifying or overriding the alphabetical order of entries (when the "author" and "editor" fields are missing). Note that this is very different from the key (mentioned just after this list) that is used to cite or cross-reference the entry.
BibTeX key is defined as follows:
In addition, each entry contains a key (Bibtexkey) that is used to cite or cross-reference the entry. This key is the first item in a BibTeX entry, and is not part of any field
- Biber offers the
idfield and refers to the bibtex key as "citatoin key" ("Citation key aliases for the main citation key.") - citation key is more common than cite key
Since "citation key" is longer than the old Cite key", I will undo this renaming and keep CiteKey.
|
LGTM 👍 |
- reorder methods in BibEntry to follow id, cite key, type, everything else - add some JavaDoc - remove obsolete setting of changed=true
|
Same here. Looks good. @koppor: Is this ready to merge or do you want to implement more improvements? |
koppor
removed
the
status: ready-for-review
rename CiteKey to BibTeXKey to be consistent with wording of JabRef userschanged=true