{"action":"created","issue":{"url":"https://api.github.com/repos/nutofem/nuto/issues/188","repository_url":"https://api.github.com/repos/nutofem/nuto","labels_url":"https://api.github.com/repos/nutofem/nuto/issues/188/labels{/name}","comments_url":"https://api.github.com/repos/nutofem/nuto/issues/188/comments","events_url":"https://api.github.com/repos/nutofem/nuto/issues/188/events","html_url":"https://github.com/nutofem/nuto/pull/188","id":295585568,"number":188,"title":"Pde dynamic poly visualization","user":{"login":"TTitscher","id":12712945,"avatar_url":"https://avatars0.githubusercontent.com/u/12712945?v=4","gravatar_id":"","url":"https://api.github.com/users/TTitscher","html_url":"https://github.com/TTitscher","followers_url":"https://api.github.com/users/TTitscher/followers","following_url":"https://api.github.com/users/TTitscher/following{/other_user}","gists_url":"https://api.github.com/users/TTitscher/gists{/gist_id}","starred_url":"https://api.github.com/users/TTitscher/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/TTitscher/subscriptions","organizations_url":"https://api.github.com/users/TTitscher/orgs","repos_url":"https://api.github.com/users/TTitscher/repos","events_url":"https://api.github.com/users/TTitscher/events{/privacy}","received_events_url":"https://api.github.com/users/TTitscher/received_events","type":"User","site_admin":false},"labels":[{"id":446024869,"url":"https://api.github.com/repos/nutofem/nuto/labels/cleanup","name":"cleanup","color":"006b75","default":false},{"id":407493044,"url":"https://api.github.com/repos/nutofem/nuto/labels/enhancement","name":"enhancement","color":"84b6eb","default":true}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-02-08T16:48:20Z","updated_at":"2018-02-09T12:05:40Z","closed_at":null,"author_association":"COLLABORATOR","pull_request":{"url":"https://api.github.com/repos/nutofem/nuto/pulls/188","html_url":"https://github.com/nutofem/nuto/pull/188","diff_url":"https://github.com/nutofem/nuto/pull/188.diff","patch_url":"https://github.com/nutofem/nuto/pull/188.patch"},"body":"__TL/DR__:\r\n~~~cpp\r\n// Before:\r\nVisualizer<AverageHandler> visu(cells, AverageGeometryQuad());\r\n\r\n// After:\r\nVisualizer visu(cells, AverageGeometryQuad());\r\n~~~\r\n\r\nDetailed:\r\n\r\nChanges the `Visualizer` to take a `HandlerInterface` instead of a template `THandler`. For the case of `AverageHandler` and `VoronoiHandler` those handlers are created internally if you provide a `AverageGeometry` or a `VoronoiGeometry`. \r\n\r\nI think the separation of the `Visualizer` and the `Handler` is great, but IMO we do not have to _pay_ the compile time cost. Plus, the interface is a tiny bit shorter, as shown above. With the prototype pattern (`HandlerInterface::Clone()`) you are still free to define custom handlers in you main files.\r\n\r\nA __new feature__ is `Visualizer::SetCells()` that allows you to redefine the cells. This is handy if you implement your problem as a class and the `Visualizer` is a member of such class. In this case, the cells have to be available in the constructor of your class, which might cause problems. Now, you can initialize the `Visualizer` with an empty cell group (`mVisualizer({}, AverageGeometryLine())`) and change them later.\r\n\r\n_PS:_ The diff in the unit tests is somehow messed up. It is basically a moving and renaming. "},"comment":{"url":"https://api.github.com/repos/nutofem/nuto/issues/comments/364416366","html_url":"https://github.com/nutofem/nuto/pull/188#issuecomment-364416366","issue_url":"https://api.github.com/repos/nutofem/nuto/issues/188","id":364416366,"user":{"login":"Psirus","id":1187292,"avatar_url":"https://avatars1.githubusercontent.com/u/1187292?v=4","gravatar_id":"","url":"https://api.github.com/users/Psirus","html_url":"https://github.com/Psirus","followers_url":"https://api.github.com/users/Psirus/followers","following_url":"https://api.github.com/users/Psirus/following{/other_user}","gists_url":"https://api.github.com/users/Psirus/gists{/gist_id}","starred_url":"https://api.github.com/users/Psirus/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Psirus/subscriptions","organizations_url":"https://api.github.com/users/Psirus/orgs","repos_url":"https://api.github.com/users/Psirus/repos","events_url":"https://api.github.com/users/Psirus/events{/privacy}","received_events_url":"https://api.github.com/users/Psirus/received_events","type":"User","site_admin":false},"created_at":"2018-02-09T12:05:39Z","updated_at":"2018-02-09T12:05:39Z","author_association":"COLLABORATOR","body":"1. I agree that the visualizer should not have a list of possible handlers. a) this *almost* amounts to making the handler and the geometry one again, and b) \"why should I pay the compile time costs\" ;)\r\n2. `SetCells` is a good idea.\r\n3. *static* vs *dynamic* handlers: I don't have a strong opinion on whether we should do the handler statically or dynamically; I originally choose static because the implementation was easier (to me). However, if we do decide to go the dynamic route, I'd like to know how much of a difference the \"OMG, so much compile\" really is. Have you benchmarked it? What is the run-time cost of a dynamic solution? Does the gain justify rewriting it? (Not that that is important anymore, since you already have).\r\n\r\nBut in the end, I'm willing to accept the PR, because the work is done and the compile time will go down (even though we don't know by how much or whether this is the only way to achieve this, but anyway...)"}}
{"actor":{"display_login":"Psirus"}}