Interview question - what's wrong with this code?

NB: This post is followed up here.

You can learn a lot about good code from reading bad code, and at least something about how well someone codes from what they can tell you about bad code. With this in mind I handed a print out of some bad code to the latest prospective developers I've interviewed, to see what they made of it. The bad code is below; I'll write another blog with the things I think is wrong with it soon (there's quite a lot of it), but it'd be very interesting to hear what people think is wrong with it before I do :)

The bad code - it doesn't actually compile, but that's the tip of its 'badness' iceberg...

namespace MyNamespace
    using System;

    public class Customer

        public void PlaceOrder(string orderReference, OrderedProductData[] orderedProductData)
            if (orderReference = "")
                throw new OrderCreationException();
            orderReference = orderReference.Trim();
            Order newOrder = new Order(orderReference);
            newOrder.Customer = this;
            int i;
            for (OrderedProductData dataItem in orderedProductData) {
                Product product = sqlserverdatabase.findproduct(dataItem.ProductID);
            "A new order has been placed: " + orderReference + " at " + DateTime.Now);
                "New Order!",
                "A new order has been placed: " + orderReference + " at " + DateTime.Now,
                "", "");

sqlserverdatabase, LoggingService and CommunicationService are all classes which exist, but are not shown.

So what's wrong with it? :)

Edit : I've had a couple of good responses to this, with things I hadn't considered - I'll approve all the comments when I post my take on it. Keep 'em coming! :)

Edit 2: I've now posted the follow up to this.

Print | posted @ Wednesday, August 3, 2011 7:47 PM

Comments on this entry:

No comments posted yet.

Post A Comment