Skip to content
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

WIP: initial commit for overpass wrapper with builder pattern #1

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

WIP: initial commit for overpass wrapper with builder pattern #1

wants to merge 19 commits into from

Conversation

wolfhardfehre
Copy link

@wolfhardfehre wolfhardfehre commented Nov 15, 2016

  • four main query builder classes ComplexQuery, NodeQuery, WayQuery and RelationQuery
  • three option builder classes for
    -- header settings (Settings)
    -- regional search requests (SearchRegion)
    -- filter tags (Tags)
  • six sign specific enums to define
    -- verbosity level (Verbosity)
    -- geometries (GeometryType)
    -- comparison filters for tags (Filter)
    -- possible regional search types (SearchType)
    -- input specific selection types (Selection)
    -- output specific modifier (Modifier)
  • two additional statement classes to allow recursion (Recursion) and building resulting sets (Sets)
  • three base type as interfaces and abstract classes to give the four main classes and the additional statements an architecture

- four main query builder classes ComplexQuery, NodeQuery, WayQuery and RelationQuery
- three option builder classes for header settings (Settings), regional search requests (SearchRegion) and for filter tags (Tags)
- six sign specific enums to define verbosity level (Verbosity), geometries (GeometryType), comparison filters for tags (Filter), possible regional search types (SearchType), input specific selection types (Selection) and output specific modifier (Modifier)
- two additional statement classes to allow recursion (Recursion) and building resulting sets (Sets)
- three base type as interfaces and abstract classes to give the four main classes and the additional statements an architecture
@wolfhardfehre
Copy link
Author

This is just work in progress. README still needs to be adjusted to the new style and some examples should be presented. I also didn't test the OverpassResponse yet if it fits all the verbosity levels and geometry types.

@johnjohndoe
Copy link
Owner

johnjohndoe commented Nov 17, 2016

Great 👍 I think I will have time to review it within the next week.

@johnjohndoe
Copy link
Owner

Sorry, I am still too busy. Do not take this personally.

Copy link
Owner

@johnjohndoe johnjohndoe left a comment

Choose a reason for hiding this comment

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

Thank you for the huge and great pull request. As we already spoke in person about this you know that I was taking an almost infinite time to review it. I hope you find my comments somewhat helpful. Please take your time to go through them. I will be happy to reply to any questions you have concerning them.

Test syntax

Did you see that I added AssertJ as a dependency? You might want to use it to make assertions more readable. Example:

Assert.assertEquals(expected, actual);

becomes:

assertThat(actual).isEqualTo(expected);

for (double coordinate : coordinates) {
coo.add(String.valueOf(coordinate));
}
return String.join(delimiter, coo);
Copy link
Owner

Choose a reason for hiding this comment

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

Replace with StringUtils.join.

* @return String
*/
public String create() {
return String.join("", tags);
Copy link
Owner

Choose a reason for hiding this comment

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

Replace with StringUtils.join.

tail.add(String.valueOf(limit));
}

return String.join(" ", tail);
Copy link
Owner

Choose a reason for hiding this comment

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

Replace with StringUtils.join.

* COUNT Get the element count.
*/
public enum Modifier {
BB("bb"),
Copy link
Owner

Choose a reason for hiding this comment

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

As you know I am not into the internals over the overpass query language. But as a library user I would love to see an a bit more verbose name for this enum. Maybe BBOX or even BOUNDING_BOX. In the end it should match what is defined in SearchType.

package info.metadude.java.library.overpass.models.query.enums;

/**
* Degree of verbosity, default is body.
Copy link
Owner

Choose a reason for hiding this comment

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

Where does this enum class define a default?

LIKE("\"~\""),
NOTLIKE("\"!~\"");

public String sign;
Copy link
Owner

Choose a reason for hiding this comment

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

Add final.

BACKWARD_WAY("bw"),
BACKWARD_RELATION("br");

public String sign;
Copy link
Owner

Choose a reason for hiding this comment

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

Add final.

IDS("ids"),
TAGS("tags");

public String sign;
Copy link
Owner

Choose a reason for hiding this comment

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

Add final.

GEOM("geom"),
COUNT("count");

public String sign;
Copy link
Owner

Choose a reason for hiding this comment

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

Add final.

/**
* Tag request clauses (or "tag filters")
*/
public enum Filter {
Copy link
Owner

Choose a reason for hiding this comment

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

For readability reasons I suggest adding _ in the enum values such as NOT_EQUAL instead of NOTEQUAL.

@wolfhardfehre
Copy link
Author

wolfhardfehre commented Nov 19, 2017

Still Todo

  • check what is wrong with travis
  • check responses with node, way, relation and complex query, probably write TypeAdapter
  • check responses for different modifiers
  • check responses for different verbosity levels

@wolfhardfehre
Copy link
Author

@johnjohndoe I still have two major problems.

The first one is related to testing, I can't use the test suite integrated into Android Studio and have to test always just with ./gradlew test. But this is not as verbose as the Studio integration. Does it also happen to you? The error is related to Android Studio not finding a TestSuite (maybe because there is no androidTest folder). When I run then ./gradlew test it seams to cache this result and when I run afterwards the specific test with Android Studios Run it gives me the cached result back. If I then change the Test it still shows the last ./gradlew test result. I think that could be more comfortable but I have my work around.

The second problem has no work around. I don't know how to fix the following Travis error
ERROR: JAVA_HOME is set to an invalid directory: /usr/lib/jvm/java-7-oracle
Where is this happening? Is it on server side? Can you fix it or tell me what needs to be changed?

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

Successfully merging this pull request may close these issues.

2 participants