{"action":"created","issue":{"url":"https://api.github.com/repos/OPM/opm-parser/issues/795","repository_url":"https://api.github.com/repos/OPM/opm-parser","labels_url":"https://api.github.com/repos/OPM/opm-parser/issues/795/labels{/name}","comments_url":"https://api.github.com/repos/OPM/opm-parser/issues/795/comments","events_url":"https://api.github.com/repos/OPM/opm-parser/issues/795/events","html_url":"https://github.com/OPM/opm-parser/issues/795","id":153637287,"number":795,"title":"Difficult api: EclipseDeck?","user":{"login":"joakim-hove","id":1321665,"avatar_url":"https://avatars.githubusercontent.com/u/1321665?v=3","gravatar_id":"","url":"https://api.github.com/users/joakim-hove","html_url":"https://github.com/joakim-hove","followers_url":"https://api.github.com/users/joakim-hove/followers","following_url":"https://api.github.com/users/joakim-hove/following{/other_user}","gists_url":"https://api.github.com/users/joakim-hove/gists{/gist_id}","starred_url":"https://api.github.com/users/joakim-hove/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/joakim-hove/subscriptions","organizations_url":"https://api.github.com/users/joakim-hove/orgs","repos_url":"https://api.github.com/users/joakim-hove/repos","events_url":"https://api.github.com/users/joakim-hove/events{/privacy}","received_events_url":"https://api.github.com/users/joakim-hove/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2016-05-08T05:44:40Z","updated_at":"2016-05-08T09:12:04Z","closed_at":null,"body":"The seemingly quite simple PR: https://github.com/OPM/opm-upscaling/pull/188 is an example of a recurring problem:\r\n\r\n1. The `Deck` datatype is essentially a quite simple container of `DeckKeyword` instances, the `Deck` container gives no upfront guarantees of the content of the container.\r\n\r\n2. The `EclipseState` constructor requires a valid[1] deck as input, and will raise an exception if e.g. the `GRID` section is missing.\r\n\r\n3. The `EclipseGrid( Deck )`constructor is by construction accepts a non valid grid - in particular a grid without a `GRID` section.\r\n\r\nWe therefor have a situation where the two constructors:\r\n\r\n```\r\nEclipseGrid::EclipseGrid( Deck ) \r\n...\r\nEclipseState::EclipseState( Deck, )\r\n```\r\nhave different semantic requirement to the input argument - that is quite difficult to see!\r\n\r\n**EclipseDeck**\r\n\r\nAs a possible improvement I suggest that we create a class e.g. `EclipseDeck` which wraps/extends an existing `Deck` instance and enforces deck validity in the constructor:\r\n\r\n```\r\nEclipseDeck::EclipseDeck( const Deck& deck) {\r\n    if (!deck.hasKeyword(\"GRID\"))\r\n      throw std::invalid_argument(\"Missing GRID section\");\r\n   ...\r\n}\r\n```\r\n\r\nBy using the `EclipseDeck` or `Deck` type in the api we can clearly communicate whether we want a semantically correct Eclipse deck, or just a random collection of keywords.\r\n\r\n\r\n\r\n[1]: Valid is quite poorly defined in this context, but it at least requires some sections. Going along the suggested route with `EclipseDeck` we could also give a much better definition to *valid deck*. "},"comment":{"url":"https://api.github.com/repos/OPM/opm-parser/issues/comments/217704971","html_url":"https://github.com/OPM/opm-parser/issues/795#issuecomment-217704971","issue_url":"https://api.github.com/repos/OPM/opm-parser/issues/795","id":217704971,"user":{"login":"andlaus","id":1837735,"avatar_url":"https://avatars.githubusercontent.com/u/1837735?v=3","gravatar_id":"","url":"https://api.github.com/users/andlaus","html_url":"https://github.com/andlaus","followers_url":"https://api.github.com/users/andlaus/followers","following_url":"https://api.github.com/users/andlaus/following{/other_user}","gists_url":"https://api.github.com/users/andlaus/gists{/gist_id}","starred_url":"https://api.github.com/users/andlaus/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/andlaus/subscriptions","organizations_url":"https://api.github.com/users/andlaus/orgs","repos_url":"https://api.github.com/users/andlaus/repos","events_url":"https://api.github.com/users/andlaus/events{/privacy}","received_events_url":"https://api.github.com/users/andlaus/received_events","type":"User","site_admin":false},"created_at":"2016-05-08T09:12:04Z","updated_at":"2016-05-08T09:12:04Z","body":"couldn't these \"correctness requirements\" be simply folded into the parseMode with EclipseState doing the parser creation and deck parsing internally?\r\n\r\nalso, IMO it would be beneficial if `EclipseState` would provide a `getDeck()` method: it seems to be a common pattern in C++ that higher-level classes provide a \"I-know-what-I'm-doing,-give-me-the-low-level-stuff-in-all-its-gory\" method; e.g. std::sting::c_str() and std::sting::data(). why should `EclipseState` stay different in that respect?"}}