Localize list tabs' routes #157
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| package org.mamute.list; | ||
|
|
||
| public class Tab { | ||
| private final Type type; | ||
| private final String localizationKey; | ||
| private final String link; | ||
|
|
||
| public Tab(Type type, String localizationKey, String link) | ||
| { | ||
| this.type = type; | ||
| this.localizationKey = localizationKey; | ||
| this.link = link; | ||
| } | ||
|
|
||
| public String getLocalizationKey() { return this.localizationKey; } | ||
| public String getLink() { return this.link; } | ||
| public Type getType() { return this.type; } | ||
|
|
||
| public enum Type { | ||
|
There was a problem hiding this comment. I would make the type separated from the Tab class(it could be within the same package with package-private access modifier) There was a problem hiding this comment. Sorry for this, as explained I'm pretty new to Java world so I thought nesting classes is considered okay. I'll move it to a separate file. |
||
| VOTED("voted"), ANSWERED("answered"), VIEWED("viewed"); | ||
|
|
||
| private final String stringValue; | ||
|
|
||
| Type(String stringValue) { | ||
| this.stringValue = stringValue; | ||
| } | ||
|
|
||
| public String getStringValue() { | ||
| return this.stringValue; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| package org.mamute.list; | ||
|
|
||
| import br.com.caelum.vraptor.http.route.Router; | ||
| import net.vidageek.mirror.dsl.Mirror; | ||
| import org.apache.commons.lang.WordUtils; | ||
| import org.mamute.controllers.ListController; | ||
|
|
||
| import javax.inject.Inject; | ||
| import java.lang.reflect.Method; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| import static java.util.Arrays.asList; | ||
|
|
||
| public class TabsHelper { | ||
|
There was a problem hiding this comment. I personally don't like this approach, seems like you are separating behaviour from attributes, making the Tab model anemic. There was a problem hiding this comment. Well the stuff implemented here is not a model business logic per-se, it is rather a mapping of the information model shouldn't know about. Think of a database model doing its own database connection. You would usually abstract that out and inject the values into the model once you fetch them. It doesn't mean that you introduced a anemic model by doing this. Again, your repo, your rules, so let me know if you insist on this :) There was a problem hiding this comment. In that case I could create a ConnectionProvider class or so, which is specific enough for others to know what it is supposed to do. My point is just that I don't like helper classes :) |
||
|
|
||
| private final Router router; | ||
|
|
||
| @Inject | ||
| public TabsHelper(Router router) { | ||
| this.router = router; | ||
| } | ||
|
|
||
| public Tab tabForType(Tab.Type tabType) { | ||
| String localizationKey = "menu.top." + tabType.getStringValue(); | ||
| String methodName = "top" + WordUtils.capitalize(tabType.getStringValue()); | ||
| return new Tab(tabType, localizationKey, urlForMethod(methodName)); | ||
|
There was a problem hiding this comment. This could be a static factory method at Tab
There was a problem hiding this comment. I considered this actually. What I didn't want to do is create a DOM with a lot of horizontal dependencies (e.g. the router dependency). Figuring out a controller method responsible for specific tag in the tag itself feels even more scary as this introduces upwards dependency between low level DOM and controller. It's your repo, so I can merge the Helper and Tab if you insist, but if feels wrong? There was a problem hiding this comment. Again, I'm just trying to eliminate this helper class. Of course you could create a factory or so for that if you don't want to put that in the model(which is fair enough) |
||
| } | ||
|
|
||
| public ArrayList<Tab> getTabs() { | ||
| ArrayList<Tab> tabs = new ArrayList<Tab>(); | ||
|
|
||
| for (Tab.Type tabType : asList(Tab.Type.values())) { | ||
| tabs.add(tabForType(tabType)); | ||
| } | ||
|
|
||
| return tabs; | ||
| } | ||
|
There was a problem hiding this comment. This could also be at Tab There was a problem hiding this comment. see comment above |
||
|
|
||
| private String urlForMethod(String method) { | ||
| return this.router.urlFor(ListController.class, method(ListController.class, method)); | ||
| } | ||
|
|
||
| private Method method(Class<?> clazz, String method) { | ||
| return new Mirror().on(clazz).reflect().method(method).withAnyArgs(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| package org.mamute.list; | ||
|
|
||
| import br.com.caelum.vraptor.http.route.Router; | ||
| import org.junit.Before; | ||
| import org.junit.Test; | ||
| import org.mamute.controllers.ListController; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import static org.junit.Assert.*; | ||
| import static org.mockito.Mockito.mock; | ||
| import static org.mockito.Mockito.when; | ||
|
|
||
| public class TabsHelperTest { | ||
|
|
||
| private Router router; | ||
|
|
||
| @Before | ||
| public void setup() { | ||
| router = mock(Router.class); | ||
| } | ||
|
|
||
| @Test | ||
| public void tabForType_creates_tab_with_correct_values() throws NoSuchMethodException { | ||
| when(this.router.urlFor(ListController.class, ListController.class.getMethod("topAnswered"))).thenReturn("/answer-link"); | ||
| TabsHelper subject = new TabsHelper(this.router); | ||
|
|
||
| Tab tab = subject.tabForType(Tab.Type.ANSWERED); | ||
|
|
||
| assertEquals(tab.getLink(), "/answer-link"); | ||
| assertEquals(tab.getLocalizationKey(), "menu.top.answered"); | ||
| assertEquals(tab.getType(), Tab.Type.ANSWERED); | ||
| } | ||
|
|
||
| @Test | ||
| public void getTabs_returns_all_tabs_in_correct_order() throws NoSuchMethodException { | ||
| when(this.router.urlFor(ListController.class, ListController.class.getMethod("topAnswered"))).thenReturn("/answer-link"); | ||
| when(this.router.urlFor(ListController.class, ListController.class.getMethod("topViewed"))).thenReturn("/viewed-link"); | ||
| when(this.router.urlFor(ListController.class, ListController.class.getMethod("topVoted"))).thenReturn("/voted-link"); | ||
| TabsHelper subject = new TabsHelper(this.router); | ||
|
|
||
| List<Tab> tabs = subject.getTabs(); | ||
|
|
||
| assertEquals(tabs.size(), 3); | ||
| assertEquals(tabs.get(0).getType(), Tab.Type.VOTED); | ||
| assertEquals(tabs.get(1).getType(), Tab.Type.ANSWERED); | ||
| assertEquals(tabs.get(2).getType(), Tab.Type.VIEWED); | ||
| } | ||
| } |
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.
Any reasons to remove the section include?
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.
I haven't seen it would be used anywhere, I'll double check this, thanks