From: Rob V. <rv...@do...> - 2013-03-02 17:05:06
|
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> |