Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix preconditions and generic types (#1) #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yeldarby
Copy link

The preconditions prevented reading row, column, and channel 0. And the lack of a type constraint on the generic classes were preventing from creating an extension that performed arithmetic operations on the pixel data.

(eg this would fail because yo ucouldn't compare image[r, c, 0] to 0; constraining <T> to a BinaryInteger should fix it (and it wouldn't even make sense for <T> to be something weird like a UIView or something like that):

extension SGLImage {
	func toASCII() -> String {
		let image = self

		var output = ""
		for r in 0..<image.height {
			if(r % 10 != 0) { continue }
			for c in 0..<image.width {
				if(c % 10 != 0) { continue }
				
				if(image[r, c, 0] > 0) {
					output += "+"
				} else {
					output += " "
				}
			}
			output += "\n"
		}

		return output
	}
}

(Fixes #2)

The preconditions prevented reading row, column, and channel 0. And the lack of a type constraint on the generic classes were preventing from creating an extension that performed arithmetic operations on the pixel data.

(eg this would fail because yo ucouldn't compare `image[r, c, 0]` to 0; constraining `<T>` to a `BinaryInteger` should fix it (and it wouldn't even make sense for `<T>` to be something weird like a `UIView` or something like that):
```
extension SGLImage {
	func toASCII() {
		let image = self

		var output = ""
		for r in 0..<image.height {
			if(r % 10 != 0) { continue }
			for c in 0..<image.width {
				if(c % 10 != 0) { continue }
				
				if(image[r, c, 0] > 0) {
					output += "+"
				} else {
					output += " "
				}
			}
			output += "\n"
		}
	}
}
```
@jkolb
Copy link
Member

jkolb commented Jan 16, 2018

I've only been updating this library but haven't actually used it yet, still checking on those conditions to figure out what those methods are for to tell if they are broken (I agree they appear so on first looking at them).

I like the idea of constraining the types, but I think FixedWidthInteger might be a better candidate as it covers pretty much the same set of types but has more operations on it which may come in handy.

@jkolb
Copy link
Member

jkolb commented Jan 16, 2018

This comment:

Only Elements UInt8, UInt16, and Float are supported internally

seems to indicate that Float is a valid data type that could be used so constraining to just integers may be bad. Currently it is not easy to constrain for both integer and floating points types, that may be why it has no constraints at all.

I've looked more closely and I think the preconditions should change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reading the first row/column/channel
2 participants