Skip to content

Commit

Permalink
Add questions to start tall.
Browse files Browse the repository at this point in the history
  • Loading branch information
Dzmitry Shalukhau committed Feb 23, 2022
1 parent c9a9ed7 commit bc9fc71
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 1 deletion.
4 changes: 4 additions & 0 deletions QUESTIONS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
* Why are you building custom cache instead of using some in-memory db (Redis, Memcached, etc)
* Ideally docker in development builds application inside container. This way allows to not install any development tools when you develop. Can you describe steps, how to implement it.
* Why you are using spring.jpa.open-in-view=false in configuration? Why developers forced to set it? Why it is antipattern? (Same questions for defer-datasource-initialization: true)
* Tests... What tests MUST be written (at least)
1 change: 1 addition & 0 deletions src/main/java/com/atanava/locator/AuthUser.java
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @QUESTION: Why are you using this package? */
package com.atanava.locator;

import com.atanava.locator.model.User;
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/com/atanava/locator/config/CacheConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@

@Slf4j
@RequiredArgsConstructor
/**
* @QUESTION: Why you don't proxy beans?
*/
@Configuration(proxyBeanMethods = false)
public class CacheConfig<K, V> {

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/atanava/locator/model/Role.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.atanava.locator.model;

import org.springframework.security.core.GrantedAuthority;

/** @QUESTION: Why did you not set ROLE_* using constructor */
public enum Role implements GrantedAuthority {
USER,
ADMIN;
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/atanava/locator/service/PointTo.java
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @QUESTION: Why it is here? */
package com.atanava.locator.service;

import com.atanava.locator.model.PointId;
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/atanava/locator/web/OsmController.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public ResponseEntity<ArrayNode> getByAddress(@RequestParam(required = false) St
@RequestParam(required = false) Integer addressdetails,
@RequestParam(required = false) String format) {

/** @QUESTION: May be content negatiation will be better??? */
format = getFormatOrDefault(format);
ArrayNode created;
if (q != null) {
Expand Down

1 comment on commit bc9fc71

@atanava
Copy link

Choose a reason for hiding this comment

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

  • Why are you building custom cache instead of using some in-memory db (Redis, Memcached, etc)

It was a test task, and we agreed that it must be hand-made cache, because they wanted to see some algorithmic thinking.

  • Ideally docker in development builds application inside container. This way allows to not install any development tools when you develop. Can you describe steps, how to implement it.

To be honest - I'm still new to docker, so it's still hard for me to answer this question.

  • Why you are using spring.jpa.open-in-view=false in configuration? Why developers forced to set it? Why it is antipattern?

This can have a big performance impact because database queries are running without transactions in auto-commit mode, and the n+1 problem can also occur.

Same questions for defer-datasource-initialization: true

Because I use schema created by Hibernate, but data.sql script runs before schema is created. I use this option to allow Hibernate to create schema first.

Tests... What tests MUST be written (at least)

OsmController (using SpringMockMVC), OsmService, JsonUtil, UrlBuilder

Why are you using this package?

It's my mistake, because previously I named my project as "locator", after that I renamed it to "geo-locator", but unfortunately forgot to rename the package.

Why you don't proxy beans?

To improve performance because these methods are only called once and they are not called from the application, the application only uses the beans.
spring-projects/spring-boot#9068
Although, for such a small application, this is probably overkill...

Why did you not set ROLE_* using constructor

Because theoretically a user can have many different roles, so I decided to use a collection that is added through the constructor using an lombok's annotation @AllArgsConstructor.

Why it is here?

In this way I wanted to use a composite primary key because the latitude and longitude pairs are always unique, but it didn’t work out well, I agree with this, it would be better to use ordinary constraints.

May be content negatiation will be better???

Perhaps content negatiation will be better, but I tried to implement my API to be a bit similar with Nominatim API

Please sign in to comment.