Need to refractor this piece of shit

Been given this absolute piece of shit to attempt to refractor
-have i missed anything?

using System;
using System.Collections;
using System.Collections.Generic;
using System.Text;

namespace Assignment1
{
    class Program
    {
        static void recieptHeader()
        {
            Console.WriteLine("\nAUTOMATED FUEL DELIVERY SYSTEM");
            Console.WriteLine("\nRECEIPT");
        }

        private static void RegisteredUsers(ArrayList registeredUsers)
        {
            registeredUsers.Add(new CommunicationSystem("476541", "Visa", 50));
            registeredUsers.Add(new CommunicationSystem("302354", "MasterCard", 400));
            registeredUsers.Add(new CommunicationSystem("470614", "Amex", 200));
            registeredUsers.Add(new CommunicationSystem("711251", "Diners", 90));
        }

        static void Main(string[] args)
        {
            SystemController aSystem = new SystemController();

            ArrayList registeredUsers = new ArrayList();

            // populate registered users
            RegisteredUsers(registeredUsers);

            aSystem.setRegisteredUsers(registeredUsers);


            // Add fuel to tank
            FuelTank aTank = aSystem.getFuelTank();
            aTank.addFuel(1000); // litres

            //validate transaction

            CardReader customer1 = aSystem.getCardReader();
            customer1.readCard("476541", "Visa");

            aSystem.getCardStatus(customer1);


            // Scenario 1 - Enter cash limit
            if (aSystem.getCardReader().getCardSatus() == SystemController.CardStatus_Valid)
            {
                Pump aPump = customer1.getPump();
                // Fuel selection
                aPump.setFuelType(SystemController.Unleaded);
                // set cash
                aPump.setPrice(40);      
                // check credit limit
                // check fuel price with customers selection
                if (customer1.getCreditLimit() > aPump.getPrice())
                {
                    double fuelDelivery = aPump.getPrice() / SystemController.UnleadedPrice;
                  
                    aPump.activate();
                    if (aTank.getTotalLevel() > fuelDelivery)
                    {
                        aPump.setFuelDispensed(fuelDelivery);
                    }
                    aPump.deactivate();
                    aTank.removeFuel(fuelDelivery);
                    customer1.setCreditLimit(customer1.getCreditLimit() - aPump.getPrice());
                    DateTime currTime = DateTime.Now;

                    // Print receipt
                    recieptHeader();
                    Console.WriteLine("\nDate : {0:d}", currTime);
                    Console.Write("\nPaid ${0} for {1:#.##} litres of ", aPump.getPrice(), fuelDelivery);
                    if (aPump.getFuelType() == SystemController.Unleaded)
                        Console.Write("Unleaded");
                    else if (aPump.getFuelType() == SystemController.PremiumUnleaded)
                        Console.Write("Premium Unleaded");
                    else if (aPump.getFuelType() == SystemController.LPG)
                        Console.Write("LPG");
                    else if (aPump.getFuelType() == SystemController.Diesel)
                        Console.Write("Diesel");
                    Console.WriteLine("\n\nPress any key to continue...");
                    string garbage = Console.ReadLine();
                    Console.Clear();
                  
                   
                }
         

                // max amount of fuel
                CardReader customer2 = new CardReader();
                customer2.readCard("302354", "MasterCard");
                aSystem.getCardStatus(customer2);

                // Scenario 2 - Customer enters max fuel
                if (aSystem.getCardReader().getCardSatus() == SystemController.CardStatus_Valid)
                {
                    // check fuel price with customers selection
                   
                    double maxDelivery = customer2.getCreditLimit() / SystemController.PremiumUnleadedPrice;
                    aSystem.setMaxDelivery(maxDelivery);
                    Pump bPump = customer2.getPump();
                    bPump.setFuelType(SystemController.PremiumUnleaded);
                    bPump.activate();
                    double fuelDispensed = 0;
                    double charge = 0;
                    bool filled = false;
                    Console.WriteLine("\n\nDISPENSING FUEL");
                    Console.WriteLine("\nPress y to start dispensing or ..\n\npress any other key to quit");
                    string a = Console.ReadLine().ToLower();
                    if (a.Equals("y"))
                    {
                        Console.Clear();
                        Console.WriteLine("\n\nDispensing fuel");
                        Console.WriteLine("\nPress <CR> to dispence in 0.1 litre increment\n\nPress q to terminate dispensing");
                        string req = Console.ReadLine().ToLower();
                        while ((aTank.getTotalLevel() >= maxDelivery) & (!req.Equals("q")) & fuelDispensed <= maxDelivery & customer2.getCreditLimit() >= charge)
                        {
                            filled = true;
                            charge += 0.1 * SystemController.PremiumUnleadedPrice;
                            fuelDispensed += 0.1;
                            aTank.removeFuel(0.1);
                                                      
                            Console.Clear();
                            Console.WriteLine("\n\nDispensing fuel");
                            Console.WriteLine("\nFuel dispensed {0:F2} litre(s) and Cost {1:C}", fuelDispensed, charge);
                            Console.WriteLine("\nPress <CR> to despence in 0.1 litre increment\n\nPress q to terminate dispensing");
                            string ch = Console.ReadLine().ToLower();
                            if (ch.Equals("q"))
                            {
                                bPump.setFuelDispensed(fuelDispensed);
                                bPump.setPrice(charge);
                                break;
                            }
                        }
                        if (filled)
                        {
                            customer2.setCreditLimit(customer2.getCreditLimit() - charge);
                            DateTime currTime = DateTime.Now;
                            // Print receipt
                            recieptHeader();
                            Console.WriteLine("\nDate : {0:d}", currTime);
                            Console.Write("\nPaid ${0} for {1:#.##} litres of ", bPump.getPrice(), bPump.getFuelDispensed());
                            if (bPump.getFuelType() == SystemController.Unleaded)
                                Console.Write("Unleaded");
                            else if (bPump.getFuelType() == SystemController.PremiumUnleaded)
                                Console.Write("Premium Unleaded");
                            else if (bPump.getFuelType() == SystemController.LPG)
                                Console.Write("LPG");
                            else if (bPump.getFuelType() == SystemController.Diesel)
                                Console.Write("Diesel");
                            Console.WriteLine("\n\nPress any key to continue...");
                            string garbage = Console.ReadLine().ToLower();
                            Console.Clear();

                        }
                    }                                       
                }
            }
        }
    }
}

the if(a.Equals("y")) statement should be put into a new method called StartDispenser(), as that is possibly something you may wish to call if the program got bigger and you needed to call it at times other than when the user presses "y".

The idea of refactoring is to create more reusable code.

Look for anything that you think might need to be called on its own.

A good idea is to create a flow chart of what is happening and try to pick out possible verbs which would denote potential methods or even classes (if you want to create new classes).

For example, just a quick look tells me you could have:
StartDispenser()
ChargeCustomer()
as potentially reusable methods

cheers mate

>>2
The idea of refactoring is to create more reusable code.
The idea of refactoring is to create simpler code.

It might involve creating reusable code if you identified several places where roughly the same thing is done, and you can extract the gist of it into a reusable method or class. Or if there are several logically separated actions/levels of action, and you want to separate them in the code, to make it easier to understand.

More often refactoring involves rewriting code in a way tailored to this particular application: initially you wrote some stuff in a reusable fashion, then it turned out that you don't need to reuse it after all, and now you can specialize and inline it and remove tons of cruft caused by unnecessary abstraction.

Look for anything that you think might need to be called on its own.
Worst advice ever. When a cat is bored, it licks its balls, Java programmers would do a great service to themselves and the humanity by learning a thing or two from cats.

>>4
The idea of refactoring is actually to make reusable code.

Why would you want to make reusable methods in a program that doesn't reuse them??  Because of System Extendability.

So, you don't reuse method "A"?  So you get rid of it right?
Well, what happens if you decide you need to extend your system and you realise that you NOW do have to reuse method "A".  Now you have a shit load of recoding to do, and possibly a harder job because you've now built a system around this code, when if you had made it reusable as part of your initial design, you'd have a lot less hastle on your hands.

Systems rarely stay the same size forever.  They change, they get bigger, they do more complex stuff.

It pays to have a system that you can extend easily, as opposed to a system that is effectively set in stone and you find yourself "hacking" into already defined methods.

"A hallmark - if not the hallmark - of good object oriented design is that you can modify and
extend a system by adding code rather than hacking it.... In short, change is additive, not
invasive. Additive change is potentially easier, more localized, less error-prone, and ultimately
more maintainable than invasive change."
John Vlissides, The C++ Report, February 1998

>>5
I'll expand my answer incase the guy from >>4 comes back with more drivel.  I'll leave a link that you can read to understand the idea of extendable software better, and why it is so important at early design stages.

http://gupea.ub.gu.se/bitstream/2077/20561/1/gupea_2077_20561_1.pdf
Here is a link to the idea of making systems easier to extend by making reusable code.

>>1
hire a coder

>>1
cause you are a shit-coder

>>5
Well, what happens if you decide you need to extend your system and you realise that you NOW do have to reuse method "A".
Then you refactor it.

and possibly a harder job because you've now built a system around this code
You couldn't have done that, since it was used in one place only, by definition.

A hallmark - if not the hallmark - of good object oriented design is that you can modify and extend a system by adding code rather than hacking it.... In short, change is additive, not invasive.
A hallmark - if not the hallmark - of the terminal OOP of the brain is the irrational, unsubstantiated and completely impenetrable delusion of prescience. In fact it's even funnier than that: afflicted don't merely believe that they can accurately predict future requirements, instead they believe that they can write code that could be easily extended to accommodate future requirements without having a slightest idea what those would be. Again and again they prove themselves wrong, but somehow interpret it as a sign that they have not tried hard enough, which only strengthens they conviction. Mind boggles.

Listen here, faggot. From my extensive experience with all kinds of legacy systems written by a variety of shitty coders in a variety of shitty styles I have come to believe that there's only one characteristic of code that helps predict how hard it would be to fix or extend it: its size. Making a change to functionality spanning 500 lines is ten times as hard as making the same change to 100 lines of code.

Now there are different kinds of code of course. It's much harder to change a monolithic function which uses and reuses the same variables all over the place than a group of functions with nicely separated responsibilities -- but that division must be produced by the desire to separate responsibilities, not by retarded precognition that you might want to reuse some of the functions.

Problem is that "good object oriented design" simply doesn't exist, unless produced by the process of gradual refactoring to accommodate actual requirements as they appear, then you can look back at it and notice that it's nice and modular and allows easy extensibility.

When, on the other hand, OOP fagdrizzles like you try to produce "good OO design" in advance, by adding abstractions, extension points, customizable functionality that you don't need to use yet, and so on, you can easily blow the size of the code 10x, and make it 100x harder to actually extend because of course none of your extension points quite fit the new requirements. And then you look at your 10000 lines of code and think -- well, the problem is that it's not customizable enough, I have not added an IOC AOP VisitorFactoryFactoryFactoryAdapter here, where I really want it now. While you could have done it as simple as possible with 1000 LOC and it would have been 100x easier to extend when and how you wanted it.

>>8
>>8
>>7
>>7


No shit dick head....If I thought i was a coder i wouldnt be asking for help

I want nothing to do with this piece of shit but i dont have much of a choice. I have never looked at c# in my life, i am into databases

>>10
c# is easier than vbs shithead, please don't touch another keyboard ever, stick to flipping burgers or giving head to random strangers.

>>9
You are a fool and a retard.

You can build a system around something that is used in one place.  You obviously, however, have not, because you are an inexperienced idiot who probably deals with tiny little systems made by script kiddies.

And no it is not possible to predict all future requirements, but you can predict a hell of a lot with good design and documentation.

I have a degree in Software Engineering and I've worked on many large systems for many years.

Enjoy creating bug filled piles of shit that can't be extended.  Because that is all you are going to create.

>>12
Stop putting yourself through all that stress shithead,
things are simple, just not simple enough for your dumb ass, bitch

>>6'
>.pdf
People need to stop perpetuating this shit format. If you got something to say, use MARKEDUP text if you really need fancy formatting, plain text otherwise.

>>12
You can build a system around something that is used in one place.

Well, yes, and if it doesn't work so well then you can improve that thing until it does (which you can do easily because it's all in one place instead of being split into fifty AdapterVisitorFactory's), and when it works all right, you'd better preserve its interface even if you see the point in refactoring the internals.

but you can predict a hell of a lot with good design and documentation.
I can predict some, you are right. You can't predict shit, based on your practical advice here, and on the fact that you advise OP (obviously an idiot) to blindly try to make stuff reusable.

I've worked on many large systems for many years.
Oh, of that I have no doubt. Large systems grow like toadstools after an August rain in presence of fagdrizzles like you. That's my point, and I find it depressing that you somehow manage to interpret it as a sign of your ability and of the correctness of your approach -- that humongous piles of OOP shit you produce is at least somewhat manageable.

A real story: an in-house log filtering application, about 15KLOC of highly OOP C++, with factories, adapters, visitors and whatnot, at least they didn't use IoC. Slow like shit, eats memory like you mom eats KFCs, buggy as hell and absolutely impenetrable for any kind of extension. Because you spend five hours trying to find what exactly do you need to subclass and which traits you need to implement, only to discover that the particular thing you want is impossible to do because of some of the hundreds design decisions.

So I duplicated the entire functionality in ~150 lines of Python, including HTML and XML output. Fast, obviously no bugs, and trivially extensible in every conceivable way, because if your core logic is fifty lines of straightforward code, you can do anything to it, you can trivially rewrite it completely in half an hour if it turns that you need some unexpected stuff. Did I mention that it was about three times faster than the original pile of shit?

>>15
>One system I worked on was rubbish and I managed to make it work in ~150 lines of code
>Now I think all OOP programs are shit
>Now I don't want to use OOP even though it is advised in almost every single University and every good programming language incorporates it

And on top of all that drivel, you haven't even offered one published piece of work that agrees with your rediculous ramblings.

I offered a piece of work from a University.  I bet you haven't even read up on good programming practices, you just pull them out of your fat ass.

>huuurrrr I know how to program because I am teh elite programmor

web monkey that does a lot of maintenance chiming in; i've seen some shitty code, but there's two things which bug me most:

- code where the control path is all over the place and i need to go into fifty files to figure out what's going. this code is usually a huge "super extensible" gang-of-four-approved pile of classes upon classes upon classes. Think symfony. The developers have envisioned precise ways in which you can extend the application (if you invest the time to read the manuals), but if you want to do something they didn't anticipate you're in deep shit.

- code consisting of huge functions and undocumented ad-hoc data structures. Think drupal. when i want to reuse some functionality, sometimes there's no other way but to copy paste it out of a function because eg you can't validate a data structure without saving it to the database or firing of an email or whatever. since editing the original files is not an option so this leads to code duplication.

what i prefer is code that has been written to do its thing. i don't want the writers of the original thing to design an entire abstraction layer geared towards extensibility because it means it'll be something unexpected and i'll have to learn it.

straightforward code is easiest to understand and thus modify. you know what to expect going in. and if it turns out i need some kind of abstraction somewhere i'll add it, no problem.

>>16
Now I don't want to use OOP even though it is advised in almost every single University and every good programming language incorporates it
I didn't say that, fagprecipitation.

What I did say was that your fucktarded advice about making a separate function that checks if a string equals to "y" because you might possibly need to do that check in the future identifies you as having OOP of the cranial cavity.

That is, you are exactly the kind of person who would develop a severely overengineered software shitpile, then get all proud that your 100KLOC pile of shit is at least somewhat manageable.

OOP (and abstraction in general) is good, man. It is awesome that when you have identified some pieces of repetitive code, or some isolated part, you can properly abstract it.

Just today I have to fix some code because as it happens some banks send several rejects for one transaction (one for each reason, and at least they come back to back), which should be coalesced into one internal message. And I went and implemented a one-message-deep buffer with rejection reasons accumulator as a separate class, because it's nice to encapsulate all that stuff there and just call "buffer.push(message)" (and "buffer.yield()" at the end of input) from the event handlers.

I did not do that because it seemed to be possibly useful in the future. I did not try to write a general-purpose templated one-message-deep buffer. I might, but only if it comes up again, and I could glean have enough information about what I really need from the actual use-cases. And it would be easy, because the code I have now is so simple.

I wrote the minimal, cleanest, simplest possible code that solves the problem. While you, as I can tell from your bullshit, are an architecture astronaut and the reason why we can't have nice things.

And on top of all that drivel, you haven't even offered one published piece of work that agrees with your rediculous ramblings.
http://mitpress.mit.edu/sicp/full-text/book/book.html bitch

Seriously though, read http://dreamsongs.net/Files/PatternsOfSoftware.pdf

Also, what kind of naive idiot are you to blindly trust anything coming from the Academia on topic of software development practices? Are you also an Austrian school libertarian?

I see no unit tests here. Why on earth are you even considering refactoring without them?

1)write tests
2)test the old program
3)delete the program
4)write the new one until test pass
5) die

>>12
Hoo boy, an appeal to academic credentials on the Internet.  Thanks for letting all of us that were still in doubt that we should ignore you. Anyway, you should consider the words of the uber-academic, Hoare, subject:

There are two ways of constructing a software design: One way is to make it so simple that there are obviously no deficiencies and the other way is to make it so complicated that there are no obvious deficiencies.

From his Turing Award lecture.

bampu pantsu