2

I am having a problem while retrieving data from an ArrayList and displaying them into textboxs. I am getting an error: Unable to cast object of type 'Lab_9.Book' to type 'Lab_9.Magazine'. I tried use 2 foreach loops but that seems to be out of the question. How could I avoid this problem?

Problem occurs here:

    // Displaying all Book objects from pubs ArrayList.
    foreach (Book list in pubs)
    {
        bookNumber++; // Count books from the begining.

        // Displaying and formating the output 
        // in txtBookList textbox.
        bookList.txtBookList.Text +=
            "=== Book " + bookNumber + " ===" + Environment.NewLine +
            list.Title + Environment.NewLine +
            list.getAuthorName() + Environment.NewLine +
            list.PublisherName + Environment.NewLine +
            "$" + list.Price + Environment.NewLine
            + Environment.NewLine;
    }

Regards.

HelpNeeder
  • 6,383
  • 24
  • 91
  • 155

3 Answers3

3

this will allow you to only have a single loop, but it's not pretty

foreach (object publication in pubs)
{
   var book = publication as Book;
   var magazine = publication as Magazine;
   if (book != null) {
     //it's a book, do your thing
   } else if (magazine != null) {
     //it's a magazine, do your thing
   } else {
    throw new InvalidOperationException(publication.GetType().Name + " is not a book or magazine: ");
   }
}

Instead of inheritance, you really want to define an interface that encapsulates the common properties and methods of all publications.

public interface IPublication
{
  string Title {get;set;}
  float Price {get;set;}
  // etc.
}

Next have your classes implement the interface

public class Book : IPublication
{
  public string Title {get;set;}
  public float Price {get;set;}
  //the rest of the book implementation
}

public class Magazine: IPublication

{
  public string Title {get;set;}
  public float Price {get;set;}
  //the rest of the magazine implementation
}

you've got a lot more flexibility at this point, but for your purposes you can keep the rest of the code as is and use just a single loop that is much cleaner, and more inline with jwJung's solution

foreach (var publication in pubs.OfType<IPublication>())
{
  // publication is either a book or magazine, we don't care we are just interested in the common properties
  Console.WriteLine("{0} costs {1}",  publication.Title, publication.Price);
}
Jason
  • 15,915
  • 3
  • 48
  • 72
  • This is basically the solution I originally suggested. OP didn't want to reformat his class structure (thus the edit to my answer). I truly think this way to go, rather than relying on a poor program structure and working around it. +1 – Paul Zaczkowski Nov 03 '11 at 06:37
  • I like this. I haven't use the interfaces much. Very educating :) But as for right now I guess I cant use this as homework but will be useful in my final project. – HelpNeeder Nov 03 '11 at 06:39
  • 1
    Using interfaces provides a lot of flexibility. If you are interested in diving into the topic i would suggest learning about Dependency Injection (DI), this question on SO is a great place to start: http://stackoverflow.com/questions/130794/what-is-dependency-injection – Jason Nov 03 '11 at 06:40
  • @Jason: Thanks, I am sure I will look this over. – HelpNeeder Nov 03 '11 at 06:44
  • 1
    I'd use an abstract base class instead of an interface here. http://msdn.microsoft.com/en-us/library/scsyfw1d(v=vs.71).aspx – mpen Nov 03 '11 at 06:55
  • that's guidance from 2003 for vb.net, i wouldn't put too much stock in it – Jason Nov 03 '11 at 07:01
2

The items of your pub object(ArrayList) are Object type instances for Book and Magazine types. So you need to filter using .OfType() to display each type. This method will returns only the instances of the target type(T) in ArrayList.

foreach (Book list in pubs.OfType<Book>())
{
}

foreach (Magazine list in pubs.OfType<Magazine>())
{
}

To combine the two foreach, I will suggest you to override ToString() or to make a base class having the string property. For using the base class, set all of values(eg title + "," + bookNumber...) to the string property. I will show you overriding ToString().

internal class Book
{
    public override string ToString()
    {
        return "=== Book " + bookNumber + " ===" + Environment.NewLine +....;
    }
}

internal class Magazine
{
    public override string ToString()
    {
        return "=== Publication: " + bookNumber + ....;
    }
}

And then you can combine the two loop.

foreach (string item in pub)
{
    Console.WriteLine(item.ToString());
}
Jin-Wook Chung
  • 4,196
  • 1
  • 26
  • 45
  • This looks great and work with out causing a problem. Another question. Is there a way of integrating both object into one foreach loop? By saying this I mean that if I could read all elements from an array the way they were written into. – HelpNeeder Nov 03 '11 at 06:18
  • Seems like a great solution. I will need that for my final project. Thanks a lot. But the problem is that I have used the class diagram and professor asks us rather to not to change anything. – HelpNeeder Nov 03 '11 at 06:36
  • @HelpNeeder: You haven't changed the inheritance or anything by adding an extra method to the class, unless you mean you're not allowed to alter the classes at all. Err..wait. Just noticed you're not using inheritance. You *should* be. – mpen Nov 03 '11 at 06:40
  • 1
    If so, use the overriding ToStrig() than making a base class. Making the base class should modify class design, but for the overriding ToString(), we do not say that design(diagram) is changed. Here, for the diagram I understood as class design. – Jin-Wook Chung Nov 03 '11 at 06:41
  • 1
    @HelpNeeder: Not sure what you're trying to say there. It looks to me like you're treating both Books and Magazines as Publications, which both have authors and book numbers. That suggests a certain amount of commonality. They should probably both derive from something called Publication, and instead of storing them in an array of objects, you should store them in an array (or list) of Publications. Ideally. Not sure if that goes against the requirements. **Edit:** What Jason said. Although I'd use an abstract base class, not an interface. – mpen Nov 03 '11 at 06:48
  • 1
    @Mark: Sorry I have worded it incorrectly. Publication class is my base class as you did mention. Book and Magazine classes are derived classes. You are absolutely correct. – HelpNeeder Nov 03 '11 at 06:52
  • @HelpNeeder: Oh.. well, good then :) You should make `pubs` a `List` then. – mpen Nov 03 '11 at 06:58
1

Edit: I've re-read your question, and your comment (sorry about that), I think what you're looking for this:

foreach (var list in pubs)
{
    if(list is Book)
    {
        Book tmp = (Book)list;
        // Print tmp.<values> 
    }

    if(list is Magazine)
    { 
        Magazine tmp = (Magazine)list;
        // Print tmp.<values>         
    }
} 
Paul Zaczkowski
  • 2,848
  • 1
  • 25
  • 26
  • I have used the class diagram which points out all elements of this file. This file doesn't need to inherit anything. Is there a way to modify this loop so it would work for my scenario? – HelpNeeder Nov 03 '11 at 06:15
  • You have solved my 'do-it-in-one-loop' question. Thank you. Works with out modifying a structure and one loop works perfect. – HelpNeeder Nov 03 '11 at 06:42