[prev in list] [next in list] [prev in thread] [next in thread]
List: squeak-dev
Subject: [squeak-dev] Re: The Trunk: JSON-cmm.62.mcz
From: Chris Muller <asqueaker () gmail ! com>
Date: 2024-02-28 4:43:01
Message-ID: CANzdToEFwH3N7-_4B5VTq0G2Jt=aE_OhZdtAAL0OAoo37PTsqg () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
HI Levente,
> > Chris Muller uploaded a new version of JSON to project The Trunk:
> > > http://source.squeak.org/trunk/JSON-cmm.62.mcz
> > <http://source.squeak.org/trunk/JSON-cmm.62.mcz>
> > >
> > > ==================== Summary ====================
> > >
> > > Name: JSON-cmm.62
> > > Author: cmm
> > > Time: 24 February 2024, 1:21:44.121899 am
> > > UUID: 8a043f53-c158-4a2a-b29a-0fb20d53d6a9
> > > Ancestors: JSON-cmm.61
> > >
> > > - Prefix json extension methods to indicate they're intended for
> > Json outputs.
> > > - Better comments, implementations and tests for new path
> > accessing functions.
> >
> > That's a bit better, though I still don't think a Bitset, Matrix or a
> > Heap should understand any of those methods.
> >
> >
> > Would you like #jsonPrintOn: removed for the same reason? There's also
> > the matter of Object>>#asJsonString, and many many other examples of
> > methods which are specific to some-but-not-all subclasses, being
> > implemented in a common superclass which the applicable ones do inherit
> > from.
>
> Let's keep the discussion on the new features you just added,
I thought I was. First you said that Matrix shouldn't understand #atPath:,
so I renamed it to #jsonAtPath: to match with #jsonWriteOn:.
Classes are a subclass of Behavior. Behavior's should be concerned with
managing their behavior database (method dictionary) and creating
instances, and that's about it. Making them do application behaviors like
#jsonAtPath:, to use your polite phrase, "feels wrong".
Application behavior is meant to be handled by the responsible object's
instance methods, not the instance of its Class (a Behavior).
I offered a way we could delete the unwanted behaviors with
#shouldNotImplement, but you ignored it. Smalltalk inheritance isn't
perfect, but that's how things are done.
> not
> existing features the community agreed on when adding the JSON package
> to Trunk.
I'm a member of the community or, at least currently, a member of the
Core-Dev group. I've been using my own derivative of your Json package you
graciously maintained in your library long before it went into trunk,
keeping my enhancements to myself. Now that we've adopted Json into the
trunk, I feel like it should belong to all of us, but this reaction tells
me you don't want to set it free. It's understandable, but also feels like
others' enhancements are not welcome if they happen to rub you the wrong
way, regardless of merit.
> > I strongly prefer delegation of responsibility over utility methods,
> > which are often not discovered because people aren't looking for them on
> > various class-sides. I prefer instead to delete behaviors with
> > #shouldNotImplement. I support if you wish to use that on methods on
> > various subclasses to delete unwanted behaviors.
> >
> > I suggested something like the following to resolve that:
> >
> > Json atPath: #('foo' 1) of: aJsonObject
> >
> > instead of
> >
> > aJsonObject atPath: #('foo' 1)
> >
> > What do you think?
> >
> > > - Fix and a test an Array of Associations to be a Json object.
> >
> > That is not a fix but a brand new feature. What is your use case for
> > that?
> >
> >
> > It's both. Because I'd Squeak to have the elegance of writing:
>
> It's just feels wrong to call a new feature a fix.
>
I felt it was a fix from the sense that the case exhibited by
#testNonDictionaryJsonObject should never have blown up before.
> > {'a' -> 1.
> > 'b' ->
> > { 'foo' -> 3.
> > 'bar' -> 4 } }
> >
> > to create a Json object with code, instead of being required to remember
> > and write out its internal Smalltalk representation (which has nothing
> > to do with the Json spec), over and over again, as in:
> >
> > {'a' -> 1.
> > 'b' ->
> > { 'foo' -> 3.
> > 'bar' -> 4 } as: OrderedDictionary } as: OrderedDictionary
>
> JsonObject is already there to create json objects.
> Your example should be written as:
>
> JsonObject new
> a: 1;
> b: (JsonObject new
> foo: 3;
> bar: 4).
> There's no need to introduce a new DSL, especially an inferior one.
I respect your desire to lock the implementation into Dictionary and
below. But please be fair, my example exactly matching Json syntax is
unquestionably syntactically superior to yours for both reading and writing.
I was never a fan of JsonObject. The override of #doesNotUnderstand: means
you want to send messages to it as if it's an object instead of just a
piece of data. But because it inherits from Dictionary, it will never be
an object. For the way you want to use JsonObject, it would've been better
to inherit from Object and wrap its own dictionary, so that user classes
could at least inherit from it where people can feel comfortable adding
their own behaviors. As it is, it's a 3am production data-issue nightmare
waiting to happen, as illustrated by this example:
self assert:
(JsonObject new
type: 'shirt';
size: 'M') size = 'M' "Assertion failed"
Maybe you believe you can "keep up with it" with more and more complicated
overrides but, why? It's just accessors! The code of Json objects should
be concerned with the domain of JSON, only! It should have API's for
accessing attributes, paths, filters, etc. Maybe this philosophical
difference about usage is where my path-accessing feature is giving you
heartburn. I think it's dangerous to treat JsonObject's as domain
objects. That's why I only ever treat it as a piece of data, and always
override the default 'dictionaryClass' to Dictionary, and wrap it in my own
domain object with the behaviors that access it with #at:, etc.
- Chris
[Attachment #5 (text/html)]
<div dir="ltr"><div dir="ltr">HI Levente,</div><br><div \
class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">> > \
Chris Muller uploaded a new version of JSON to project The Trunk:<br> > \
> <a href="http://source.squeak.org/trunk/JSON-cmm.62.mcz" rel="noreferrer" \
target="_blank">http://source.squeak.org/trunk/JSON-cmm.62.mcz</a><br> > \
<<a href="http://source.squeak.org/trunk/JSON-cmm.62.mcz" rel="noreferrer" \
target="_blank">http://source.squeak.org/trunk/JSON-cmm.62.mcz</a>><br> > \
><br> > > ==================== Summary ====================<br>
> ><br>
> > Name: JSON-cmm.62<br>
> > Author: cmm<br>
> > Time: 24 February 2024, 1:21:44.121899 am<br>
> > UUID: 8a043f53-c158-4a2a-b29a-0fb20d53d6a9<br>
> > Ancestors: JSON-cmm.61<br>
> ><br>
> > - Prefix json extension methods to indicate they're intended \
for<br> > Json outputs.<br>
> > - Better comments, implementations and tests for new path<br>
> accessing functions.<br>
> <br>
> That's a bit better, though I still don't think a Bitset, Matrix \
or a<br> > Heap should understand any of those methods.<br>
> <br>
> <br>
> Would you like #jsonPrintOn: removed for the same reason? There's also \
<br> > the matter of Object>>#asJsonString, and many many other examples of \
<br> > methods which are specific to some-but-not-all subclasses, being <br>
> implemented in a common superclass which the applicable ones do inherit <br>
> from.<br>
<br>
Let's keep the discussion on the new features you just added, \
</blockquote><div><br></div><div><div>I thought I was. First you said that Matrix \
shouldn't understand #atPath:, so I renamed it to #jsonAtPath: to match with \
#jsonWriteOn:.</div><div><br></div><div>Classes are a subclass of Behavior. \
Behavior's should be concerned with managing their behavior database (method \
dictionary) and creating instances, and that's about it. Making them do \
application behaviors like #jsonAtPath:, to use your polite phrase, "feels \
wrong". Application behavior is meant to be handled by the responsible \
object's instance methods, not the instance of its Class (a \
Behavior).</div><div><br></div><div>I offered a way we could delete the unwanted \
behaviors with #shouldNotImplement, but you ignored it. Smalltalk inheritance \
isn't perfect, but that's how things are done.</div><div> \
<br></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">not <br> existing \
features the community agreed on when adding the JSON package <br> to \
Trunk.</blockquote><div><br></div><div>I'm a member of the community or, at least \
currently, a member of the Core-Dev group. I've been using my own derivative of \
your Json package you graciously maintained in your library long before it went into \
trunk, keeping my enhancements to myself. Now that we've adopted Json into the \
trunk, I feel like it should belong to all of us, but this reaction tells me you \
don't want to set it free. It's understandable, but also feels like \
others' enhancements are not welcome if they happen to rub you the wrong way, \
regardless of merit.</div><div> </div><blockquote class="gmail_quote" \
style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"> > I strongly prefer delegation of \
responsibility over utility methods, <br> > which are often not discovered because \
people aren't looking for them on <br> > various class-sides. I prefer \
instead to delete behaviors with <br> > #shouldNotImplement. I support if you \
wish to use that on methods on <br> > various subclasses to delete unwanted \
behaviors.<br> > <br>
> I suggested something like the following to resolve that:<br>
> <br>
> Json atPath: #('foo' 1) of: aJsonObject<br>
> <br>
> instead of<br>
> <br>
> aJsonObject atPath: #('foo' 1)<br>
> <br>
> What do you think?<br>
> <br>
> > - Fix and a test an Array of Associations to be a Json object.<br>
> <br>
> That is not a fix but a brand new feature. What is your use case for<br>
> that?<br>
> <br>
> <br>
> It's both. Because I'd Squeak to have the elegance of writing:<br>
<br>
It's just feels wrong to call a new feature a \
fix.<br></blockquote><div><br></div><div>I felt it was a fix from the sense that the \
case exhibited by #testNonDictionaryJsonObject should never have blown up \
before.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > \
{'a' -> 1.<br> > 'b' -><br>
> { 'foo' -> 3.<br>
> 'bar' -> 4 } }<br>
> <br>
> to create a Json object with code, instead of being required to remember <br>
> and write out its internal Smalltalk representation (which has nothing <br>
> to do with the Json spec), over and over again, as in:<br>
> <br>
> {'a' -> 1.<br>
> 'b' -><br>
> { 'foo' -> 3.<br>
> 'bar' -> 4 } as: OrderedDictionary } as: \
OrderedDictionary<br> <br>
JsonObject is already there to create json objects.<br>
Your example should be written as:<br>
<br>
JsonObject new<br>
a: 1;<br>
b: (JsonObject new<br>
foo: 3;<br>
bar: 4). </blockquote><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"> <br>
There's no need to introduce a new DSL, especially an inferior \
one.</blockquote><div><br></div><div><div>I respect your desire to lock the \
implementation into Dictionary and below. But please be fair, my example exactly \
matching Json syntax is unquestionably syntactically superior to yours for both \
reading and writing.</div></div><div><br></div><div>I was never a fan of JsonObject. \
The override of #doesNotUnderstand: means you want to send messages to it as if \
it's an object instead of just a piece of data. But because it inherits from \
Dictionary, it will never be an object. For the way you want to use JsonObject, it \
would've been better to inherit from Object and wrap its own dictionary, so that \
user classes could at least inherit from it where people can feel comfortable adding \
their own behaviors. As it is, it's a 3am production data-issue nightmare \
waiting to happen, as illustrated by this example:</div><div><br></div> self \
assert: </div><div class="gmail_quote"> (JsonObject new<br> \
type: 'shirt';<br><div> size: 'M') size = \
'M' "Assertion failed" <br></div><div><br></div><div>Maybe \
you believe you can "keep up with it" with more and more complicated \
overrides but, why? It's just accessors! The code of Json objects should be \
concerned with the domain of JSON, only! It should have API's for accessing \
attributes, paths, filters, etc. Maybe this philosophical difference about usage is \
where my path-accessing feature is giving you heartburn. I think it's dangerous \
to treat JsonObject's as domain objects. That's why I only ever treat it as \
a piece of data, and always override the default 'dictionaryClass' to \
Dictionary, and wrap it in my own domain object with the behaviors that access it \
with #at:, etc.</div><div><br></div><div> - Chris</div><div> </div></div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic