contact: fold the factory into the constructor
Contact.for guarded a door that had no lock —
new stayed public the whole time and walked around the block.
The keywords and the raise moved into initialize instead:
one way in, six call sites, and the factory is dead.
Assisted-by: Claude Opus 5 via Claude Code
diff --git a/lib/pro_tacts/contact.rb b/lib/pro_tacts/contact.rb
index 54ce75b..ef451e8 100644
--- a/lib/pro_tacts/contact.rb
+++ b/lib/pro_tacts/contact.rb
@@ -102,31 +102,30 @@ module ProTacts
:line,
)
- # A contact from its id, its stored card, its birthday, and the
- # content lines it inherits from its groups. The only way to make
- # one: an etag that came from anywhere but the card in hand is an
- # etag that can be wrong, and a birthday or an inheritance from
- # anywhere but the store is wrong the same way — which is why
- # `birthday:` and `inherited:` are required and carry no defaults,
- # since an optional nil would let a caller quietly get no birthday
- # off a card that has one, and an omitted inheritance would serve a
- # member its group's lines no longer reach. Named `stored:` rather
- # than `vcard:` because what #vcard returns is composed, and an
- # argument that is not what the reader hands back is a trap.
- #: (id: String, stored: VCard, birthday: Birthday?, inherited: Array[Inherited]) -> Contact
- def self.for(id:, stored:, birthday:, inherited:)
- raise ArgumentError, "invalid contact id: #{id}" unless id.match?(ID_FORMAT)
-
- new(id, stored, birthday, inherited)
- end
-
#: (VCard vcard) -> String
def self.etag_for(vcard)
%("#{Digest::SHA256.hexdigest(vcard.to_s)}")
end
- #: (String id, VCard stored, Birthday? birthday, Array[Inherited] inherited) -> void
- def initialize(id, stored, birthday, inherited)
+ # A contact from its id, its stored card, its birthday, and the
+ # content lines it inherits from its groups. An etag that came from
+ # anywhere but the card in hand is an etag that can be wrong, and a
+ # birthday or an inheritance from anywhere but the store is wrong
+ # the same way — which is why `birthday:` and `inherited:` are
+ # required and carry no defaults, since an optional nil would let a
+ # caller quietly get no birthday off a card that has one, and an
+ # omitted inheritance would serve a member its group's lines no
+ # longer reach. Named `stored:` rather than `vcard:` because what
+ # #vcard returns is composed, and an argument that is not what the
+ # reader hands back is a trap.
+ #
+ # The id check is here rather than behind a factory: an id outside
+ # ID_FORMAT cannot be served, and a guard a caller can walk around
+ # by reaching for `new` is not a guard.
+ #: (id: String, stored: VCard, birthday: Birthday?, inherited: Array[Inherited]) -> void
+ def initialize(id:, stored:, birthday:, inherited:)
+ raise ArgumentError, "invalid contact id: #{id}" unless id.match?(ID_FORMAT)
+
@id = id
@stored = stored
@birthday = birthday
@@ -182,7 +181,7 @@ module ProTacts
def own
return @own if defined?(@own)
- @own = self.class.for(id: @id, stored: @stored, birthday: @birthday, inherited: [])
+ @own = self.class.new(id: @id, stored: @stored, birthday: @birthday, inherited: [])
end
# The etag over #vcard's bytes, derived here on first ask and
diff --git a/lib/pro_tacts/store.rb b/lib/pro_tacts/store.rb
index 2dd7294..bc6cab6 100644
--- a/lib/pro_tacts/store.rb
+++ b/lib/pro_tacts/store.rb
@@ -290,7 +290,7 @@ module ProTacts
# member's submission is the write half's task
# (docs/plans/2026-08-24-vcard-storage-and-groups.md), so until it
# lands a member's rewrite carries the inherited lines within it.
- contact = Contact.for(id:, stored:, birthday:, inherited: inherited_of(id))
+ contact = Contact.new(id:, stored:, birthday:, inherited: inherited_of(id))
@database.transaction do
cards
.insert_conflict(target: :id, update: {vcard: Sequel[:excluded][:vcard], updated_at: NOW})
@@ -318,7 +318,7 @@ module ProTacts
#: (String id, String vcard, birthday: Birthday?) -> Contact
def rewrite(id, vcard, birthday:)
card = VCard.new(vcard)
- contact = Contact.for(id:, stored: card, birthday:, inherited: inherited_of(id))
+ contact = Contact.new(id:, stored: card, birthday:, inherited: inherited_of(id))
@database.transaction do
cards
.insert_conflict(target: :id, update: {vcard: Sequel[:excluded][:vcard], updated_at: NOW})
@@ -525,7 +525,7 @@ module ProTacts
# describe what a client downloads rather than the bytes on disk.
#: (Hash[Symbol, untyped] row, Birthday? birthday, Array[Contact::Inherited] inherited) -> Contact
def contact_from(row, birthday, inherited)
- Contact.for(
+ Contact.new(
id: row.fetch(:id).to_s,
stored: VCard.new(row.fetch(:vcard).to_s),
birthday:,
diff --git a/test/pro_tacts/admin/test_format.rb b/test/pro_tacts/admin/test_format.rb
index 6d74793..039b9e1 100644
--- a/test/pro_tacts/admin/test_format.rb
+++ b/test/pro_tacts/admin/test_format.rb
@@ -13,7 +13,7 @@ class FormatTest < Minitest::Test
# unmodeled BDAY beside nil, and inheritance defaults to none (see
# Contact).
def contact(vcard = CARD, id: "aiden", birthday: nil, inherited: [])
- ProTacts::Contact.for(id:, stored: ProTacts::VCard.new(vcard), birthday:, inherited:)
+ ProTacts::Contact.new(id:, stored: ProTacts::VCard.new(vcard), birthday:, inherited:)
end
# Initials come from the structured N property (given + family),
diff --git a/test/pro_tacts/test_contact.rb b/test/pro_tacts/test_contact.rb
index 29fc308..2510f50 100644
--- a/test/pro_tacts/test_contact.rb
+++ b/test/pro_tacts/test_contact.rb
@@ -31,7 +31,7 @@ class ContactTest < Minitest::Test
def contact(vcard = CARD, id: "aiden", birthday: nil, inherited: [], group: GROUP)
lent = inherited.map { ProTacts::Contact::Inherited.new(group:, line: it) }
- ProTacts::Contact.for(id:, stored: ProTacts::VCard.new(vcard), birthday:, inherited: lent)
+ ProTacts::Contact.new(id:, stored: ProTacts::VCard.new(vcard), birthday:, inherited: lent)
end
# The decode's refusal reports (see #photo); the transport this pins
diff --git a/test/pro_tacts/test_store.rb b/test/pro_tacts/test_store.rb
index a4893c6..fc12ad4 100644
--- a/test/pro_tacts/test_store.rb
+++ b/test/pro_tacts/test_store.rb
@@ -448,8 +448,9 @@ class StoreTest < Minitest::Test
# row in: if the transaction were dropped, the card would survive and a
# client's sync token would point at a history that never happened.
#
- # Refusing a bad id does not test this, however it is named: Contact.for
- # raises before the transaction opens, so nothing was ever attempted.
+ # Refusing a bad id does not test this, however it is named:
+ # Contact's constructor raises before the transaction opens, so
+ # nothing was ever attempted.
def test_a_write_that_fails_at_the_log_leaves_no_card
Dir.mktmpdir do |dir|
path = Pathname.new(dir) / "contacts.db"