Open noelwelsh opened 6 months ago
I'm happy to work on this. I could be useful to turn it into a project to submit 1 PR at the time. Can I do it or does it need to be someone with write access to the repo?
Java2dWriter
extends both FileWriter
(previously Writer
) and Base64Writer
(previously Base64
). On the other hand Java2dWriter
is a separate object that only uses one function from the Java2dWriter
companion object. I was thinking that it would make sense to:
Java2dWriterUtils
)Java2dWriter
into 2 traits: Java2dFileWriter
and Java2dBase64Writer
Writers
that stores all of the actual writer objects (e.g Java2dPdfWriter
, Java2dGifWriter
, Java2dBufferedImageWriter
)Is that reasonable or is it better to just leave it as Java2dWriter
(because technically it is missleading to call it a Java2dFileWriter
since it is also a Base64Writer
) and have another file Java2dBufferedImageWriter
?
I've created a project: https://github.com/orgs/creativescala/projects/8/
Regarding the refactorings you describe above:
Moving the companion object to a class means that code is not usable (and hence not testable) without creating an instance of the class. I'd rather not add this complication if it's not necessary. Some of the code could move into Java2d
, which handles other parts of rendering using the Java 2D API.
I'm not sure there is value in separating the implementation of Java2dWriter
. Conceptually they are distinct, and they are represented by distinct algebras / type classes. However, the implementations are related (i.e. they share a lot of the implementation.) I don't think it's a problem to have the implementations together. The concepts are already distinct, just at a different level of the code.
This is reasonable.
Consider the following:
BufferedImageConverter
is quite a mouthful. Can we shorten the name? Perhaps we use the term "Writer" for anything we can convert aPicture
into, and rename the currentWriter
toFileWriter
(andBase64
becomesBase64Writer
)?Can we make the
Frame
optional? Can we get it from aDefaultRenderer
? Maybe the idea of a default frame should be extracted into it's own effect (it's not really an effect, but it is associated with effects.)Do we need to return the value of type
A
when we convert to aBufferedImage
? Most of the time the user will throw it away. (Same issue exists forBase64
). Maybe the convention should be you get theA
if you convert to anIO
but we throw it away in you immediately run thatIO
. This would match what happens indraw
.Maybe what we need is a generic conversion effect type class. Something like
Java2dBufferedImageWriter
inJava2dWriter.scala
should be renamed and moved to its own file.Add tests.
Add documentation.
This issue could probably be usefully turned into a project. Each bullet could be it's own issue.