On 03/14/2016 08:08 PM, Peter Harpending wrote: > On 03/14/2016 05:10 PM, Bryan Richter wrote: >> 1. I'm not sure the Eq instances of MechPatron and MechProject are >> sound. As defined, `==` is the moral equivalent of checking that the >> two values cannot both exist in the database at the same time >> (database uniqueness), but that's not quite the same thing as being >> the same value. > > So, I think I redid this between when you wrote this review and > now. Now, the `Eq` instance makes sure the external keys are equivalent. > > instance Eq MechPatron where > m1 == m2 = mechPatronExternalKey m1 == mechPatronExternalKey m2 > > instance Ord MechPatron where > compare = comparing mechPatronExternalKey > > instance Eq MechProject where > m1 == m2 = mechProjectExternalKey m1 == mechProjectExternalKey m2 > > instance Ord MechProject where > compare = comparing mechProjectExternalKey > > The reason I wanted to do this is so that `compare` would return `EQ` > iff `(==)` would return `True`. I can't remember why I wanted to sort > them? I think I wanted to put them in a `Set` (from the `containers` > package), and to do any nontrivial `Set` operations, the inner type must > be an instance of `Ord`. > > `HashSet` or whatever from `unordered-containers` doesn't require the > inner type to have an `Ord` instance, however is does require a > `Hashable` instance, which might actually not be that bad. Sets in > mathematics do not require the concept of ordering, so `containers` > seems like an affront to basic mathematical decency. It's almost as bad > as there existing a `w : Word` such that `w + 1 = 0`. > > Mathematical decency aside, I'll look in to this. > >> 3. I don't think the concept of a suspended patron is necessary yet. >> I could be misremembering, but I'm pretty sure one is either a patron >> or one is not. > > This is me attempting (poorly) to think ahead. The concept of a pledge > being suspended because of insufficient funds is necessary (or we could > just delete the pledges, but that seems sort of mean). Moreover, I can > imagine that Patrons would want to prioritize the projects, so if they > can only afford projects X, Y, and Z, they can exclude project W > temporarily if they want. >
This seems to me to be an implementation detail. We can treat a pledge as "suspended" in the front-end if we simply have the information that a patron pledged, ran out of funds, and that's the current status. Thus, it's different than if they actively removed their pledge. I.e. the status can be determined purely by the history alone. It's just an implementation detail whether we mark it as having that status in the mechanism code directly or in the database. I agree that it matters to be able to distinguish between pledges where user ran out of funds vs where they chose to unpledge (whether or not they have funds remaining). The idea about users making two different versions of unpledging for temporary vs longer-term not donating is definitely not MVP and shouldn't be done at this time nor is it decided by design folks. >> 5. >> >>> he = it >> >> I get it, but it provides very little compared to the damage it >> could cause. > > What damage is that? Other than quasi-feminist stuff? Would it be better > if I wrote `he = specify`? > I don't know the context here but unless this is a weird play-on-words joke, if this is *actually* about pronouns please stick to "they" always for generic regardless of singular or plural. >> 6. The stuff I wrote for doing tests against a database was intended >> to be abstracted out into a standalone library. I'll still do that. >> I guess you had to rewrite it to be able to use HSpec, though, so >> *shrug* >> > > FWIW, your tests are still there, in the `chreekat` directory. I sort of > ported them over to HSpec, and then figured out a rather novel way to > abuse the concept of property-based testing. >
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Dev mailing list [email protected] https://lists.snowdrift.coop/mailman/listinfo/dev
