Hey,<br><div><br>Ran into this thread shortly after posting some comment here: <a href="https://gerrit.wikimedia.org/r/#/c/68952/">https://gerrit.wikimedia.org/r/#/c/68952/</a><br><br>Is it correct that the idea here is to only have this 
PropertyBadValueSnak to represent invalid PropertyValueSnaks? In other words , this does not cover the other types of snaks?<br><br>&gt; This may happen because a property is deleted<br><br>That also happens for PropertySomeValueSnak and PropertyNoValueSnak.<br>

<br>&gt; 
Data we already have in the database may become &quot;invalid&quot; by some definition.<br><br>Indeed, by <b>some definition</b>. This is what makes me uncomfortable with this proposed change. It is not clear to me how it is intended to be used and what follow up changes will be made. We should be very careful to not force code that does not care about this to hold it into account. For instance, if I got some tool that keeps a mirror of certain entities up to date, it needs to be able to fetch entities via the API, and then represent them as entity objects, and be able to write those to persistence. It would not care at all about what is considered invalid, and would not even have the info required to determine this validity at hand.<br>

<br>In case the PropertyBadValueSnak class is only useful for what we are currently doing in Wikibase Repo, then we ought to just move it there. Or to lib, if client also needs it. Since I&#39;m not sure how this is intended to be used, it&#39;s hard to say if it would be appropriate or not to put it in DataModel. It is however something to keep in mind. If it is not needed there, not putting it in decreases the hassle we&#39;ll have if it turns out we made a bad choice. Right now I only see PropertyBadValueSnak used in SnakFactory, which only contains code that does not belong in DataModel and should therefore be moved to lib. (And this should happen ASAP as it currently causes a violation of the acyclic dependency principle.)<br>

<br>&gt; Is the Snak level the right place for handling this case?<br><br>I&#39;m guessing you considered doing this on the DataValue level as well - why did you not go with that approach?<br><br>Cheers<br><br>--<br>Jeroen De Dauw<br>

<a href="http://www.bn2vs.com" target="_blank">http://www.bn2vs.com</a><br>Don&#39;t panic. Don&#39;t be evil. ~=[,,_,,]:3<br>--<br></div>