dotnetrdf-develop

Re: [dotNetRDF-Develop] Portable Class Library update

[Rob V. <rv...@do...>]

Hey Kal

Sounds excellent, comments inline:

From:  Kal Ahmed <ka...@ne...>
Date:  Thursday, February 28, 2013 2:16 AM
To:  Rob Vesse <rv...@do...>
Subject:  Portable Class Library update

> Hi Rob,
> 
> I'm slowly making progress on the PCL version of the core library. There are a
> couple of things I want to run by you to make sure I don't spend a lot of time
> hacking up the API in ways you don't like :)
> 
> 1) I'm targeting a pretty broad set of platforms, SL4+, .NET 4+, Windows Phone
> 7.1+ and Metro; rather than going for just the latest (e.g. SL5, .NET 4.5,
> WP8, Metro) the main downside I can see to that is no support for C#5 stuff
> like async/wait and maybe a few core windows APIs that are implemented in the
> later versions. 

I am happy with this, I specifically haven't used async/await anywhere in
the codebase so that we don't have to maintain two versions of the code to
work on the older and newer platforms.  Regardless async/await is just
syntactic sugar around the .Net async support.

> 
> 2) PCL has no System.IO.File implementation. I guess because the different
> platforms do file I/O differently. As far as I can see there are two choices
> here:
>   a) Create an abstraction and then implement it in a separate
> platform-specific assembly on each of the platforms (this is the approach that
> the PCLContrib folks have taken)
>   b) Remove the dependency on the File class from the portable build of
> dotNetRDF. What this means in effect is using #defines to remove all APIs that
> take a file name and leave only those that take a Stream or Reader class.
> I have opted for (b) because it will leave a smaller, cleaner API - it removes
> a bit of utility and forces the app to do the stream management and just pass
> us streams.

Yes I like (b) as an option

Are you just eliminating methods entirely in those builds or are you going
down the path of having them throw a PlatformNotSupportedException?  I don't
mind if you do the former but I think long term it would be nicer to users
if we do everything the latter way as it means the API looks identical
across platforms.  Ideally we should only be eliminating methods entirely
where they reference a type that is not available on a platform.  If they
merely use something internally that is not available we should prefer
PlatformNotSupportedException.

I realize that I am guilty of doing the former for stuff like the Storage
API so I will file myself an issue to remedy that :)

> 
> 
> 3) For some platforms async versions of APIs will probably be useful. I've not
> really made a start on that yet and I don't plan to until I have some binaries
> compiled and tested with the existing synchronous APIs - my feeling is that
> app developers can wrap the synchronous calls up if they need async for now.

Yes, there are async APIs for some obvious things but these are fairly
limited.  Longer term we should discuss what else might need an async API

> 
> 4) I have added a couple of #define symbols. PORTABLE indicates a PCL build
> (though the portable project also sets the SILVERLIGHT flag to avoid having to
> change masses of #if !SILVERLIGHT conditions). NO_FILE is used to
> conditionally drop those methods that rely on System.IO.File to open a stream.

Ok, per my earlier comment it may make more sense to comment out the
implementation and throw a PlatformNotSupportedException advising users of
the alternative API to use instead.

> 
> I still have quite a lot of typing to do to get rid of those pesky file APIs
> (and it is mostly just typing), but I hope that in a few days I'll be able to
> at push something functional into my clone of the source tree.
> 
> If you have strong feelings about the file APIs or about the need for adding
> async versions of APIs, now would be a good time to let me know :)

Bar the point on removing methods vs throwing PlatformNotSupported all
sounds good.

Do you have a rough idea when this might be ready?  It would be awesome to
get this into the 1.0.0 release if we can

Rob

> 
> Cheers
> 
> Kal
> -- 
> Kal Ahmed
> Director, Networked Planet Limited
> e: kal...@ne...
> w: www.networkedplanet.com <http://www.networkedplanet.com>

Re: [dotNetRDF-Develop] Portable Class Library update

[Rob V. <rv...@do...>]

Hey Kal

Yes I agree, I think that this is likely more developer friendly in the long
term

Rob

From:  Kal Ahmed <ka...@ne...>
Date:  Monday, March 4, 2013 11:54 AM
To:  Rob Vesse <rv...@do...>
Subject:  Re: Portable Class Library update

> Hi Rob,
> 
> Just wondered if you have any further thoughts about the API thing before I
> spend a lot of time hacking it one way or the other. The more I think about it
> the more I think it makes sense to make the PCL API a subset of the full
> dotNetRDF API and actually take out the APIs that require the File class -
> e.g. if you want to see what it takes to port code that runs on the "native"
> .NET 4.0 version of dotNetRDF over to a portable class library project for a
> metro app, then you will get a bunch of compile time errors rather than having
> to review your API usage call by call.
> 
> K
> 
> 
> On Fri, Mar 1, 2013 at 9:37 AM, Khalil Ahmed <ka...@ne...> wrote:
>> Hi Rob,
>> 
>> Thanks for the feedback. I do see what you mean about keeping the API
>> consistent. My concern is that Intellisense won't pick up on a method not
>> being implemented, which will mean that developers writing against the PCL
>> version will have to be really careful about how they use the API to avoid
>> run-time errors. I think that may be why MS decided that for their PCL stuff
>> they would just take the APIs out entirely. I'm not sure if it is possible to
>> / how to mark a method as not supported in PCL via docstrings, but if that is
>> something that can be done, that may help - so I'll have a look to see if I
>> can find anything. I'm happy to go either way on this, but I feel that the
>> developer experience might be better if PCL only exposes a subset of the API
>> rather than exposing  APIs that throw PlatformNotSupportedExceptions (because
>> there will be *a lot* of them :-)
>> 
>> In terms of timing, I'm not totally sure yet how long it will take - VS says
>> I've got about 200 compiler errors in the core library to resolve. Then I
>> guess the next step will be to get as many of the tests working with PCL as
>> the API changes allow. So there may be something that is more an untested
>> alpha than a stable library with all the tests in place. I'll hopefully have
>> a bit of time this weekend to have a bash through the remaining API changes
>> (one way or the other) and that may then give a clearer view of if its then
>> just a case of making a reduced unit test suite or if there are any more
>> fundamental issues lurking in there.
>> 
>> Cheers
>> 
>> Kal
>> 
>> 
>> On Thu, Feb 28, 2013 at 5:30 PM, Rob Vesse <rv...@do...> wrote:
>>> Hey Kal
>>> 
>>> Sounds excellent, comments inline:
>>> 
>>> From:  Kal Ahmed <ka...@ne...>
>>> Date:  Thursday, February 28, 2013 2:16 AM
>>> To:  Rob Vesse <rv...@do...>
>>> Subject:  Portable Class Library update
>>> 
>>>> Hi Rob,
>>>> 
>>>> I'm slowly making progress on the PCL version of the core library. There
>>>> are a couple of things I want to run by you to make sure I don't spend a
>>>> lot of time hacking up the API in ways you don't like :)
>>>> 
>>>> 1) I'm targeting a pretty broad set of platforms, SL4+, .NET 4+, Windows
>>>> Phone 7.1+ and Metro; rather than going for just the latest (e.g. SL5, .NET
>>>> 4.5, WP8, Metro) the main downside I can see to that is no support for C#5
>>>> stuff like async/wait and maybe a few core windows APIs that are
>>>> implemented in the later versions.
>>> 
>>> I am happy with this, I specifically haven't used async/await anywhere in
>>> the codebase so that we don't have to maintain two versions of the code to
>>> work on the older and newer platforms.  Regardless async/await is just
>>> syntactic sugar around the .Net async support.
>>> 
>>>> 
>>>> 2) PCL has no System.IO.File implementation. I guess because the different
>>>> platforms do file I/O differently. As far as I can see there are two
>>>> choices here:
>>>>   a) Create an abstraction and then implement it in a separate
>>>> platform-specific assembly on each of the platforms (this is the approach
>>>> that the PCLContrib folks have taken)
>>>>   b) Remove the dependency on the File class from the portable build of
>>>> dotNetRDF. What this means in effect is using #defines to remove all APIs
>>>> that take a file name and leave only those that take a Stream or Reader
>>>> class. 
>>>> I have opted for (b) because it will leave a smaller, cleaner API - it
>>>> removes a bit of utility and forces the app to do the stream management and
>>>> just pass us streams.
>>> 
>>> Yes I like (b) as an option
>>> 
>>> Are you just eliminating methods entirely in those builds or are you going
>>> down the path of having them throw a PlatformNotSupportedException?  I don't
>>> mind if you do the former but I think long term it would be nicer to users
>>> if we do everything the latter way as it means the API looks identical
>>> across platforms.  Ideally we should only be eliminating methods entirely
>>> where they reference a type that is not available on a platform.  If they
>>> merely use something internally that is not available we should prefer
>>> PlatformNotSupportedException.
>>> 
>>> I realize that I am guilty of doing the former for stuff like the Storage
>>> API so I will file myself an issue to remedy that :)
>>> 
>>>> 
>>>> 
>>>> 3) For some platforms async versions of APIs will probably be useful. I've
>>>> not really made a start on that yet and I don't plan to until I have some
>>>> binaries compiled and tested with the existing synchronous APIs - my
>>>> feeling is that app developers can wrap the synchronous calls up if they
>>>> need async for now.
>>> 
>>> Yes, there are async APIs for some obvious things but these are fairly
>>> limited.  Longer term we should discuss what else might need an async API
>>> 
>>>> 
>>>> 4) I have added a couple of #define symbols. PORTABLE indicates a PCL build
>>>> (though the portable project also sets the SILVERLIGHT flag to avoid having
>>>> to change masses of #if !SILVERLIGHT conditions). NO_FILE is used to
>>>> conditionally drop those methods that rely on System.IO.File to open a
>>>> stream.
>>> 
>>> Ok, per my earlier comment it may make more sense to comment out the
>>> implementation and throw a PlatformNotSupportedException advising users of
>>> the alternative API to use instead.
>>> 
>>>> 
>>>> I still have quite a lot of typing to do to get rid of those pesky file
>>>> APIs (and it is mostly just typing), but I hope that in a few days I'll be
>>>> able to at push something functional into my clone of the source tree.
>>>> 
>>>> If you have strong feelings about the file APIs or about the need for
>>>> adding async versions of APIs, now would be a good time to let me know :)
>>> 
>>> Bar the point on removing methods vs throwing PlatformNotSupported all
>>> sounds good.
>>> 
>>> Do you have a rough idea when this might be ready?  It would be awesome to
>>> get this into the 1.0.0 release if we can
>>> 
>>> Rob
>>> 
>>>> 
>>>> Cheers
>>>> 
>>>> Kal
>>>> -- 
>>>> Kal Ahmed
>>>> Director, Networked Planet Limited
>>>> e: kal...@ne...
>>>> w: www.networkedplanet.com <http://www.networkedplanet.com>
>> 
>> 
>> 
>> -- 
>> Kal Ahmed
>> Director, Networked Planet Limited
>> e: kal...@ne...
>> w: www.networkedplanet.com <http://www.networkedplanet.com>
> 
> 
> 
> -- 
> Kal Ahmed
> Director, Networked Planet Limited
> e: kal...@ne...
> w: www.networkedplanet.com <http://www.networkedplanet.com>