score:6

Accepted answer

as a quick win, you can remove the .tolist() call. all you're doing is enumerate items, so there's no need to do that. similarly, there's no need to create bookandautherlist.

eventually i think you can strip it down to this:

public ienumerable<book> getauthorwithbookname(string keyword)
{
    return from author in getauthorname(keyword)
           let book = getbook(author)
           from xmlbook in xdocument.parse(book.outerxml).descendants("books")
           select new book
           {
               authorname = author.authorname,
               bookname = xmlbook.attribute("name").value
           };
}

score:0

try this:

foreach (var bookname in booknamelist)
{
    yield return new book()
    {
        authorname = author.authorname,
        bookname = bookname
    };
}

and remove other return statement.

score:1

not sure you'll get much 'optimization'. yield is better used when you would often break part way through using the enumeration you are generating, so that you don't do unnecessary computation. but here goes:

this is untested.

public ienumerable<book> getauthorwithbookname(string keyword)
{
    foreach (var author in getauthorname(keyword))
    {
        xdocument xdocbooksnames = xdocument.parse(getbook(author).outerxml);

        var booknamelist = from x1 in xdocbooksnames.descendants("books")
                            select x1.elements("book").select(g => g.attribute("name").value);

        foreach (var bookname in booknamelist)
        {
            yield return new book()
                {
                    authorname = author.authorname,
                    bookname = bookname
                };
        }
    }
}

what i did:

  1. remove the tolist() on the expression, it already returns an enumerable. removed some code by consolidating getauthorname into the foreach (note - make sure that function yields and ienumerable too, if you can)
  2. yield return instead of adding to a list

score:9

responses by rubens and luke correctly explain the use of yield.

however, this looks suspicious to me.

xmlnode booksnames = getbook(author);
xdocument xdocbooksnames = xdocument.parse(booksnames.outerxml);

you convert xml to string and then parse it again, only because you want to convert it from dom node to xml.linq node. if you are talking about optimization, then this is much more inefficient than creating an extra list.


Related Query

More Query from same tag