Added model.entry.FieldName that contains field name constants

[oscargus]

Based on the idea that all fields should have a constant, I started to implement this. The ideas are (please comment):

• Class position and name: model.entry.FieldName
• Constant naming scheme AUTHOR_FIELD
• Only use the constants when denoting the actual fields of an entry. Strings with the same content, but different purpose are not replaced (sometimes it is not obvious though).
• The constants are not used in test.

I do not expect to convert all fields in this PR, but I wanted some comments on it before continuing further.

[tobiasdiez]

Nice idea. I would even go one step further and

• Make FieldName an enum. This would probably require to introduce overloads accepting the enum, e. g. getFieldOptional(FieldName field) { return getFieldOptional(field.name()) }. With the aim to replace strings completely using the extended enum pattern
• Introduce helper methods in BibEntry which return values for common fields, e.g. getAuthors() { return getFieldOptional(FieldName.Authors); }

[matthiasgeiger]

👍
Some remarks:

• Instead of using FieldName.AUTHOR_FIELD I would propose to omit the _FIELD suffix. FieldName.AUTHOR is less clumsy and would result in easier to read lists of field as they are used in the entry type definitions...
• What will happen to those "OR" fields such as "year/date" - directly use this as a constant? Or use a method orField(FieldName.YEAR, FieldName.DATE)?

Regarding the enum usage: Basically this would be the better approach - but as the the user can define his own "custom fields" things get rather complicated as there always must be two ways to deal with field definitions

[simonharrer]

👍 but I would just use string constants for now, being in line with the argument from @matthiasgeiger .

[tobiasdiez]

@matthiasgeiger One can use the extended enum pattern for user-defined fields. It would roughly look like:

interface FieldName { 
  String getName(), 
  List<FieldName> getAliases(),
  FieldName convertToBibLatex(),
  FieldName convertToLatex(),
  ...
}

enum StandardFieldName implements FieldName { 
  Authors("authors", ...), 
  Title("title", ...), ... 
}

class UnknownFieldName implements FieldName {
  UnknownFieldName(String name) {...}

  @Override getName() {return name; }
  @Override ...
}

[Siedlerchr]

That Extended Enum looks interesting.

2016-07-19 11:34 GMT+02:00 Tobias Diez notifications@github.com:

@matthiasgeiger https://github.com/matthiasgeiger One can use the extended
enum pattern https://blogs.oracle.com/darcy/entry/enums_and_mixins for
user-defined fields. It would roughly look like:

interface FieldName {
String getName(),
List getAliases(),
convertToBibLatex(),
convertToLatex(),
...
}
enum StandardFieldName implements FieldName {
Authors("authors", ...),
Title("title", ...), ...
}
class UnknownFieldName implements FieldName {
UnknownFieldName(String name) {...}

@OverRide getName() {return name; }
@OverRide ...
}

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#1602 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AATi5JbGFc85zPL5Hy9RjS6OqV-4uWQuks5qXJoJgaJpZM4JPM_L
.

[simonharrer]

Hm, this could work, but on the other hand, it introduces additional complexity into the model, and everybody working with the model then must learn this pattern. I would like to keep the model simple, and would at first use only such String constants.

[oscargus]

I've removed the _FIELD part for now.

Considering enum one should maybe think about InternalBibtexFields as well, creating a class/enum that can contain all information about a field (maybe including column widths etc).

Combined fields, I was thinking of just doing a string concatenation (as they are just constants in a few places), but it may make sense to create a function for this.

[oscargus]

There are two types of combined fields: one with / as separator for columns and one with ; as separator for preferences. FieldNames.orFields(List<String) and FieldNames.andFields(List<String)?

[oscargus]

I think I'm happy for now. A bit tedious and there are quite a few fields to convert... Most of the most common ones should be converted already though. May be better to merge now as it is quite boring rebasing with conflicts...

@tobiasdiez : I think adding get-methods clearly makes sense, although I also think that getAuthors() should return Optional<AuthorList>. :-)

[simonharrer]

I think this is a huge step forward, and would merge this ASAP.

[matthiasgeiger]

👍 Just merge after rebasing!

[oscargus]

I merge this now. The failing test seems to relate to a lack of internet connection rather than anything else. Also, I do not understand how to get things up and running with the new medline fetcher, but that shouldn't affect this PR...