2009-07-08, 20:11
OK, I've been working with the new version for a while now and have some initial feedback. First of all, thanks again for doing this work, it's saved me a ton of time. Please take this as constructive criticism, virtually everything on this list is a "nice to have" and not a bug, and the bugs that are there are easily fixed or worked around.
1) "Paramater" is misspelled in ScraperSetting, it should be "Parameter".
2) It would be nice for ScraperSettings to have a public default constructor and make the Deserialize method public. I'm not sure why I believe this is true anymore, but I wrote it down so at some point I thought it was interesting. This may just be an artifact of how my application worked with settings in the prior version.
3) Deserialize calls in general could use an overload with takes a string, rather than always having to construct an XElement. This would allow your callers to not have to be Linq aware.
4) It would be nice if I could specify a null as the path to the cache directory on the various scraper constructors, and you'd use the Temp directory by default. Easy to work around, but would be nice.
5) There's a small bug in the video scraper (may be in the others, I haven't checked) where if you make a GetDetails call without doing a prior CreateSearch, you get an exception due to WebpageDownload being uninitialized.
6) Polymorphism: This library could really use some. There are lots of common elements between the various types of scrapers and the various types of media tags, and it would be very useful (for me, anyway) if those could be pulled out into base classes so I could work with the data in a more general way. In my particular case, I'm working with a large library of movies and TV shows, and I'd like to avoid special-case code wherever possible. Currently I wrote a wrapper object to abstract out the common elements, but this is really something that should be in the base design, I think.
Have you done any performance testing on this version versus the previous? Qualititatively the C# version seems faster, but I wonder if that's just my perception.
1) "Paramater" is misspelled in ScraperSetting, it should be "Parameter".
2) It would be nice for ScraperSettings to have a public default constructor and make the Deserialize method public. I'm not sure why I believe this is true anymore, but I wrote it down so at some point I thought it was interesting. This may just be an artifact of how my application worked with settings in the prior version.
3) Deserialize calls in general could use an overload with takes a string, rather than always having to construct an XElement. This would allow your callers to not have to be Linq aware.
4) It would be nice if I could specify a null as the path to the cache directory on the various scraper constructors, and you'd use the Temp directory by default. Easy to work around, but would be nice.
5) There's a small bug in the video scraper (may be in the others, I haven't checked) where if you make a GetDetails call without doing a prior CreateSearch, you get an exception due to WebpageDownload being uninitialized.
6) Polymorphism: This library could really use some. There are lots of common elements between the various types of scrapers and the various types of media tags, and it would be very useful (for me, anyway) if those could be pulled out into base classes so I could work with the data in a more general way. In my particular case, I'm working with a large library of movies and TV shows, and I'd like to avoid special-case code wherever possible. Currently I wrote a wrapper object to abstract out the common elements, but this is really something that should be in the base design, I think.
Have you done any performance testing on this version versus the previous? Qualititatively the C# version seems faster, but I wonder if that's just my perception.
