It is because vmVMMakerVersion is expected to answer a number, and nil is not a number.

It's similar to what you might do for a method that is expected to answer a collection. In that case, you might choose to answer #() as a default, but it would be best not to answer nil because all senders would then need to do nil checks. By choosing #() as a default, the normal behavior will be error free and any sender that cares can check for #isEmpty.

This is not just a Smalltalk thing, the Java folks might have learned this the hard way but they have come to the same conclusion:

https://medium.com/javarevisited/just-dont-return-null-dcdf5d77128f

Dave

On 2024-01-06 19:41, christoph.thiede@student.hpi.uni-potsdam.de wrote:

Hi Dave,

why not answer nil instead of 0? I think with your change, non-VMMaker Spur VMs will start to skip all ImageSegmentTests. Maybe senders of #vmVMMakerVersion should also check it for nil or send a new check #vmIsVMMaker before?

Best, CHristoph

---

_Sent from __Squeak Inbox Talk [1]_

On 2024-01-05T02:08:55+00:00, commits@source.squeak.org wrote:

...

David T. Lewis uploaded a new version of System to project The Trunk: http://source.squeak.org/trunk/System-dtl.1442.mcz

==================== Summary ====================

Name: System-dtl.1442 Author: dtl Time: 4 January 2024, 8:07:18.703979 pm UUID: 2ef3f6e2-a4fd-4d82-a79c-6ed6f4eb05b2 Ancestors: System-ct.1441

Do not fail loading package postscript when calling upscaleDisplayOnHighDPI: if VM is not OSVM

=============== Diff against System-ct.1441 ===============

Item was changed: ----- Method: SmalltalkImage>>vmVMMakerVersion (in category 'system attributes') ----- vmVMMakerVersion

• "Answer the version number of the VMMaker package from which the 
  

main VM was compiled,

• or zero if the version number cannot be determined."
  

• "Answer the version number of the VMMaker package from which the 
  

main VM was compiled." "Smalltalk vmVMMakerVersion"

| vmMakerID |

• vmMakerID := self vmVersion substrings
  

•     detect: [:token| token beginsWith: 'VMMaker']
  

•     ifNone: [ '0' ].
  

• vmMakerID := self vmVersion substrings detect: [:token| token 
  

beginsWith: 'VMMaker']. ^Integer readFrom: (vmMakerID subStrings: '.') last!

Links: ------ [1] https://github.com/hpi-swa-lab/squeak-inbox-talk

On 2024-01-13 19:24, christoph.thiede@student.hpi.uni-potsdam.de wrote:

Hi Dave,

I would argue that answering a "somewhat plausible" default value such as zero does often do more harm than answering a clear null object because they unncessarily prolong the fault propagation chain, making it harder to debug errors. As mentioned before, your change causes to skip all ImageSegmentTests in non-VMMaker VMs. A two-valued logic like in Smalltalk vmVMMakerVersion < 2295 cannot accurately cover the absence of VMMaker. I think ImageSegmentTest class>>#testSelectors (and any other senders of #vmVMMakerVersion, analogously) should look like this instead:

testSelectors

(Smalltalk isRunningSpur and: [Smalltalk vmIsVMMaker] _"<-- added!"_ and: [Smalltalk vmVMMakerVersion < 2295]) ifTrue: "The ImageSegment Test is known to not work on older Spur OSVMs with the prospect of crashing. #expectedFailures does not cut it here; don't even try to run them" [^#()]. ^super testSelectors

Hi Christoph,

The purpose of my update is in the commit comment:

"Do not fail loading package postscript when calling upscaleDisplayOnHighDPI: if VM is not OSVM"

This was blocking the update stream for V3 images. It is not an issue when running on OSVM, but on an interpreter VM it caused a 'NotFound: Object is not in the collection.' error because the vmVMMakerVersion method is trying to parse out the substrings in system attribute 1004, which might or might not contain the string 'VMMaker'.

Usually this would be annoying but harmless, but when it happens in a package postscript, it blocks the update stream, and that is why I let vmVMMakerVersion default to 0 rather than fail outright.

This is really not very important, and if you want to revert it don't mind. But it seemed to me to be a good thing to clean up.

-1 for #vmIsVMMaker. VMMaker is the package name that is used for OSVM and also for the traditional VM, and maybe some others as well. So if you needed a test for "is opensmalltalk-vm" then maybe you would call it #isOSVM. But I don't see any real need for this. Or maybe I am misunderstanding what you mean by "non-VMMaker VMs".

Dave

Hi Christoph --

answering a clear null object

No, "nil" is not a null object. In this case, 0 would be the null object because it is part of the domain. An extra #vmIsWMMaker also contradicts with the idea of a transparent null object.

I agree with Dave here.

Best, Marcel

Am 13.01.2024 20:25:08 schrieb Thiede, Christoph christoph.thiede@student.hpi.uni-potsdam.de:

Hi Dave,

I would argue that answering a "somewhat plausible" default value such as zero does often do more harm than answering a clear null object because they unncessarily prolong the fault propagation chain, making it harder to debug errors. As mentioned before, your change causes to skip all ImageSegmentTests in non-VMMaker VMs. A two-valued logic like in Smalltalk vmVMMakerVersion < 2295 cannot accurately cover the absence of VMMaker. I think ImageSegmentTest class>>#testSelectors (and any other senders of #vmVMMakerVersion, analogously) should look like this instead:

testSelectors

(Smalltalk isRunningSpur and: [Smalltalk vmIsVMMaker] "<-- added!" and: [Smalltalk vmVMMakerVersion < 2295]) ifTrue: "The ImageSegment Test is known to not work on older Spur OSVMs with the prospect of crashing. #expectedFailures does not cut it here; don't even try to run them" [^#()]. ^super testSelectors

The article you shared was an interesting read but I think it mostly applies to typed languages like Java which handle nullability in different ways. Returning an Optional.empty() instead of null just makes sense because the compiler will warn you when you assume the former to be available but not when you assume the latter to be available (and btw, without being proficient in Java, many compilers seem to support @NotNull annotations today). Squeak does not have tools like these (because we believe in the simplicity of dynamic typing and exploratory programming instead), but perhaps the most idiomatic way to describe both cases would be a #vmWithVMMakerVersionDo:ifNotVMMaker: message. This way, senders *cannot* ignore one of these cases. Just my $0.02. :-)

Best, Christoph

--- Sent from Squeak Inbox Talkhttps://github.com/hpi-swa-lab/squeak-inbox-talk

On 2024-01-06T21:16:20+00:00, lewis@mail.msen.com wrote:

It is because vmVMMakerVersion is expected to answer a number, and nil is not a number.

It's similar to what you might do for a method that is expected to answer a collection. In that case, you might choose to answer #() as a default, but it would be best not to answer nil because all senders would then need to do nil checks. By choosing #() as a default, the normal behavior will be error free and any sender that cares can check for #isEmpty.

This is not just a Smalltalk thing, the Java folks might have learned this the hard way but they have come to the same conclusion:

https://medium.com/javarevisited/just-dont-return-null-dcdf5d77128f

Dave

On 2024-01-06 19:41, christoph.thiede(a)student.hpi.uni-potsdam.de wrote:

...

Hi Dave,

why not answer nil instead of 0? I think with your change, non-VMMaker Spur VMs will start to skip all ImageSegmentTests. Maybe senders of #vmVMMakerVersion should also check it for nil or send a new check #vmIsVMMaker before?

Best, CHristoph

---

_Sent from __Squeak Inbox Talk [1]_

On 2024-01-05T02:08:55+00:00, commits(a)source.squeak.org wrote:

...

David T. Lewis uploaded a new version of System to project The Trunk: http://source.squeak.org/trunk/System-dtl.1442.mcz

==================== Summary ====================

Name: System-dtl.1442 Author: dtl Time: 4 January 2024, 8:07:18.703979 pm UUID: 2ef3f6e2-a4fd-4d82-a79c-6ed6f4eb05b2 Ancestors: System-ct.1441

Do not fail loading package postscript when calling upscaleDisplayOnHighDPI: if VM is not OSVM

=============== Diff against System-ct.1441 ===============

Item was changed: ----- Method: SmalltalkImage>>vmVMMakerVersion (in category 'system attributes') ----- vmVMMakerVersion

• "Answer the version number of the VMMaker package from which the

main VM was compiled,

• or zero if the version number cannot be determined."

• "Answer the version number of the VMMaker package from which the

main VM was compiled." "Smalltalk vmVMMakerVersion"

| vmMakerID |

• vmMakerID := self vmVersion substrings
• detect: [:token| token beginsWith: 'VMMaker']
• ifNone: [ '0' ].

• vmMakerID := self vmVersion substrings detect: [:token| token

beginsWith: 'VMMaker']. ^Integer readFrom: (vmMakerID subStrings: '.') last!

Links:

[1] https://github.com/hpi-swa-lab/squeak-inbox-talk