Optimize Code by using yield or changing algorithm

Question

The code below works but I want to optimize the code using yield or by changing algorithm.

public IEnumerable<Book> GetAuthorWithBookName(string keyword)
{
    var bookAndAuthorList = new List<Book>();
    List<Author> AuthorNameList = getAuthorName(keyword);
    foreach (var author in AuthorNameList)
    {
        XmlNode booksNames = getBook(author);
        XDocument XDOCbooksNames = XDocument.Parse(booksNames.OuterXml);

        var bookNameList = (
            from x1 in XDOCbooksNames.Descendants("Books")
            select x1.Elements("book").Select(g => g.Attribute("name").Value))
            .ToList();
        foreach (var bookName in bookNameList)
        {
            bookAndAuthorList.Add(new Book()
            {
                authorName = author.authorName,
                bookName = bookName
            });
        }
    }
    return bookAndAuthorList;
}

public class Book
{
    public string authorName { get; set; }
    public string bookName { get; set; }
}

Answer 1

Try this:

foreach (var bookName in bookNameList)
{
    yield return new Book()
    {
        authorName = author.authorName,
        bookName = bookName
    };
}

And remove other return statement.

Answer 2

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 yeilds and ienumerable too, if you can)
2. Yeild return instead of adding to a list

Answer 3

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
           };
}

Answer 4

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.