From... methods

May 8, 2015 at 3:51 PM
Edited May 8, 2015 at 6:08 PM
Many thanks for a great library!

I am not sure if this has been discussed before, but one thing I find a bit non-intuitive with the library are the FromContent, FromStream etc. methods in the WriteableBitmapConvertExtensions.cs file.

One example is FromStream that in one overload has the following signature:
public static WriteableBitmap FromStream(this WriteableBitmap bmp, Stream stream);
Upon exit, a new WriteableBitmap is created, and this same object is also returned from the method.

I would find it more intuitive if this was not implemented as an extension method. Instead, I would recommend to move all the From methods to BitmapFactory and remove the extension argument. For example:
public static class BitmapFactory {
  public static WriteableBitmap FromStream(Stream stream) { ... }
The code can remain practically the same, only that the extension argument assignment in each method is removed, e.g. delete assignments like this from the moved methods,
bmp = new WriteableBitmap(/* BitmapImage */ bmpi);
and return the created WriteableBitmap directly instead.

What do you think?

Best regards,
Anders @ Cureos
May 12, 2015 at 7:05 PM
Just to clarify, this concerns the methods loading a WriteableBitmap from resources. The FromByteArray methods are not concerned, since the incoming bitmap is required with these methods.

I am finalizing a patch to illustrate my point, please stay tuned :-)
May 12, 2015 at 8:20 PM
Edited May 13, 2015 at 2:01 PM
I have now added a patch with ID 17504 to support this proposal. It originates from commit 113740.
Jul 22, 2015 at 6:54 AM
Has anybody reviewed the patch corresponding to this discussion item yet? I would appreciate any feedback.

Thanks in advance,
Anders @ Cureos
Jul 22, 2015 at 7:42 AM
Not yet, but I agree with you in general. It is grown ;) the From... methods were there before the BitmapFactory. That's why.
Now I'm considering moving those to the BitmapFactory but keeping the extension methods with an [Obsolete] attribute and remove those a few versions after. Hard to move stuff without breaking existing code of users. ;)
On the other hand this also breaks the consistency with the FromByteArray method. Maybe I'll keep this one in both.
Jul 22, 2015 at 8:27 AM
Thanks for responding, René.

Actually, in the patch (ID 17504) that I posted recently, I have done exactly what you are describing, i.e. keeping the extension methods but with an [Obsolete] attribute. I had problems with Subversion, so instead of a diff patch I have included the full files that I have modified. It should be a simple task to just unzip the patch archive and copy the files into the right folders.