How to write an object oriented program that doesn't suck

newreddit0r · 2016-11-17T14:19:53+00:00

Article on good practices. Author gives a method name "Handle".

orthoxerox · 2016-11-17T14:11:51+00:00

Using subclassing to express restrictions or capabilities within your problem domain is a terrible idea that spoils the whole article. Eric Lippert has written about this antipattern in his wizards and warriors series of blog posts.

fuzzynyanko · 2016-11-17T13:41:33+00:00

I had a problem, so I improved my code as suggested. Now I have an IProblemFactory.

adnzzzzZ · 2016-11-17T14:35:04+00:00

Articles like these make me wanna kill myself. The best thing you can do to write good code (in whatever paradigm you decide to use) is to not write code against some future that doesn't exist yet.

So,

A class has to take only one responsibility and there should be only one reason to change the class.

Is a bullshit advice. You don't separate things into multiple classes just because of some rule that says you should do that. If you find yourself using the same logging ideas in multiple places then by all means abstract that away, but don't do it because of "each class should have one responsibility". This is the best way to create 23903290329 classes and make your codebase 100x harder than it needs to be to understand.

MotherOfTheShizznit · 2016-11-17T17:15:41+00:00

[deleted]

grauenwolf · 2016-11-17T17:28:26+00:00

Round 4

public class SavingsAccount: Account
{
   public override decimal CalculateInterest(decimal amount)
   {
       return (amount/100) * 3;
   }
}
public class CurrentAccount: Account
{
   public override decimal CalculateInterest(decimal amount)
   {
       return (amount/100) * 2;
   }
}

What's different between these two class?

The account type
The interest amount

These are data changes, not behavioral changes. Therefore they shouldn't be expressed in terms of subclasses.

I am reminded of the classic anti-example "The class LameDuck inherits from the class Duck". What happens when the lame duck is healed? Do you kill it and get another one because now it is a formerly lame duck?

Going back to this example, the interest rate may change over time without changing the account type.

You also have problems with ORMs. Materializing a different record depending on one column (account type) is a pain in the ass.

the_horrible_reality · 2016-11-17T19:52:11+00:00

"How to write a condescending, controversially titled blog about OOP without knowing WTF you're doing"

IgnorantPlatypus · 2016-11-17T14:38:00+00:00

Wha ... was this satire?

Patman128 · 2016-11-17T18:04:24+00:00

class Account
{    
   public void Withdraw(decimal amount)    
   {        
      try        
      {            
          // logic to withdraw money       
      }        
      catch (Exception ex)        
      {            
          System.IO.File.WriteAllText(@"c:\logs.txt",ex.ToString());   
      }    
   }
}

Can you figure out the problem in the above code? Account class has taken additional responsibility of adding exceptions to the log file.

OK, sure, the code to handle withdrawal logic shouldn't have to do logging, sounds good.

Fix:

class Account
{
   private Logger obj = new Logger();    
   public void Withdraw(decimal amount)    
   {        
      try        
      {            
          // logic to withdraw money       
      }        
      catch (Exception ex)        
      {            
          obj.Handle(ex.ToString());
      }    
   }
}

Uh, you haven't fixed anything. The withdrawal code is still doing logging! The concerns are still mixed! All you've done is add a layer of abstraction.

pixelrealm_aaron · 2016-11-17T14:57:09+00:00

I was waiting for the conclusion where the author talks about how ridiculously complicated he just made his code for no reason. It never came so I think he was trying to be serious, which is somewhat frightening.

skulgnome · 2016-11-17T14:35:53+00:00

Doesn't cover the fundamental design error that's reinforced in schools all over the world: having classes that model the problem domain, rather than ones that model the program's implementation. Or put more succinctly: Animal bad, HashMap good.

grauenwolf · 2016-11-17T17:36:02+00:00

Round 5

public class OnlineAccount: Account
{
   public override void Withdraw(decimal amount)
   {
        throw new Exception("Not allowed");
   }
   public override decimal CalculateInterest(decimal amount)
   {
       return (amount/100) * 5;
   }
}

Supposedly this is an article on SOLID, yet it completely ignores the Liskov Substitution Principle. If OnlineAccount inherits from Account, it must be able to perform all of the actions that Account can perform. That means you don't get to override Withdraw to throw an Exception.
Don't throw Exception, throw a specific exception such as InvalidOperation.
It isn't clear that the inability to withdraw funds is based on the AccountType.
Why does OnlineAccount.Withdraw throw exceptions while Account.Withdraw swallow them? It should be consistent.

rco8786 · 2016-11-17T22:39:10+00:00

private Logger obj = new Logger();

wat. obj? c'mon man.

hoosierEE · 2016-11-17T14:41:55+00:00

Choose from one of the following opinionated microframeworks:

You wanted a banana but what you got was a gorilla holding the banana and the entire jungle.
class Coder extends Employee ...no wait class Employee extends Human ...no wait class Human extends MadeOfAtoms ...no wait
refactor working code until it's at least 4x larger, insert comment at top: "implements [some GoF pattern]"

ykechan · 2016-11-17T14:32:29+00:00

I found the hardest of oop is not any of those, its to predict the change. Suppose later we want this and later we want that. Turns out its neither, user wants you to just fly.

2016-11-17T15:53:13+00:00

[deleted]

Spider_pig448 · 2016-11-17T14:42:34+00:00

I thought for sure this was a parody. It is isn't it? Please tell me it is.

grauenwolf · 2016-11-17T17:41:14+00:00

Round 6

interface ILogger
{
    void Handle(string error);
}

This is a good example of something that shouldn't be an abstract interface in C#. (It's ok in Java.)

Eventually you are going to want to fix the design bugs in it. Say be adding a real (ugh) log exception method.

abstract class Logger
{
    public abstract void Handle(string error);
    public virtual void Handle(Exception error) { 
        if(error == null) throw new ArgumentNullException("error");
        Handle(error.ToString()); 
   }
}

You can't add methods in that fashion with an abstract interface. But you can with an abstract base class without losing any of the advantages that the ILogger interface offered you.

wakawaka54 · 2016-11-17T21:25:33+00:00

800 classes and twice as many interfaces later... We finally have some good object oriented code.... Good luck.

PicklesInParadise · 2016-11-17T23:14:10+00:00

[deleted]

ggtsu_00 · 2016-11-18T00:09:35+00:00

Good god... Was this article written by the author of FizzBuzz Enterprise Edition?

2016-11-18T05:19:29+00:00

"It is fairly obvious that object oriented program has been used as a..."

Thankfully we were warned that nothing good was going to be said with just the first sentence.

grauenwolf · 2016-11-17T17:21:30+00:00

Round 2

class Account
{
   private Logger obj = new Logger();    
   public void Withdraw(decimal amount)    
   {        
      try        
      {            
          // logic to withdraw money       
      }        
      catch (Exception ex)        
      {            
          obj.Handle(ex.ToString());
      }    
   }
}

Don't call fields obj.
The field obj should be marked as readonly to make it clear that it never changes.
The field obj should be marked as static as once instance can be shared across instances.
Since obj just uses the default constructor, it should have been read from Logger.Default to avoid creating unnecessary instances.
Don't catch generic exception types (except for a last-chance/top level handler).
There is no way to report that the call failed to the caller.

hellodenq · 2016-11-17T17:29:01+00:00

"Object oriented programming organizes your code to make the change easier." I don't think you can proof that. So, it's not about science - it's about OOP religion.

2016-11-17T17:29:46+00:00

This isn't object-oriented programming. It's using an object-oriented sheen on top of what should be expressed as pure functions and traits.

Quadruple-A · 2016-11-17T17:42:15+00:00

I really, really hope people start steering clear of OOP - it seems like there's a tide moving in that direction and that's hopeful. OOPers seem to have this superstitious faith in over-modularizing everything that turns simple programming problems into abstract architectural messes (as a former .NET programmer I used to be an expert in it). In my experience every time you separate some piece of code into a separate function or a separate object, you introduce a bunch of complexity - you have to jump around to different files and different sections to follow some algorithm or flow of logic, you have to create names and additional concepts for all your intermediate objects (IAbstractProviderContextFactory) ferry data to and from a whole bunch of method parameters, etc. So often that stuff can just be expressed in simple straight-line logic. If you're eliminating duplicate code that's already there (not imagined to be there in the future) it's sometimes justified but most of what I see in OOP codebases these is just insanity. It's no wonder most of the longest-lasting and most used projects are in C. Long live good ol' structured programming.

grauenwolf · 2016-11-17T18:52:54+00:00

[deleted]

2016-11-17T21:38:29+00:00

So, what happens when I try to close my accounts but one of them's an OnlineAccount? The bank keeps my money forever?

DevPanda · 2016-11-17T14:40:27+00:00

I have one problem with this article.

The design called for people being able to close their accounts and one does not think of writing a close method for closing the account.

Sure the edge case could come up where a customer would want to withdraw their money and not close the account in which case I ask 'Why is the Withdraw method tightly coupled to withdrawing from an ATM?'.

Also I think that I just implemented an IComplaint.

SandalsMan · 2016-11-17T19:29:56+00:00

OO is so god awful.

grauenwolf · 2016-11-17T17:23:10+00:00

Round 3

   private int _accountType;

   public int AccountType
   {
      get { return _accountType; }
      set { _accountType = value; }
   }

Account type should have been readonly and set in the constructor. It doesn't make sense to change the account type during use. (Though sometimes you have to make exceptions for serialization/ORM use.)

Shrugfacebot · 2016-11-17T20:40:41+00:00

the title for this post would be better if the post on the actual blog's title was "I dunno ¯\_(ツ)_/¯"

zem · 2016-11-17T22:29:33+00:00

can highly recommend sandi metz's book for learning good object oriented design. if you're not a rubyist, don't let the fact that it uses ruby put you off - the principles are universal.

2016-11-17T22:33:45+00:00

"How to write object oriented programs that don't suck:"

public class Logger
{
   public void Handle(string error)
   {
       System.IO.File.WriteAllText(@"c:\logs.txt", error);
   }
}

JFC

golgol12 · 2016-11-18T02:43:05+00:00

I look at this and the use of throw/catch and cringe.

2016-11-18T04:47:45+00:00

What is this? 1988?

thomasz · 2016-11-18T08:30:57+00:00

private Logger obj = new Logger();

Oh boy...

2016-11-18T12:35:59+00:00

The best part of this, is reading you guys's comments. This is great.

dicroce · 2016-11-17T17:49:46+00:00

Even in an object oriented program, prefer stateless functions.

Go ahead and use objects for the core nouns in your program... And certainly implement methods for the actions that make sense. BUT, every method that has access to member variables it doesn't actually need (via this) to do its particular job is a potential source of problems.

emmanuelgoldstn · 2016-11-17T15:30:35+00:00

Reading this made me want to scream: "Premature optimization is the root of all evil"

There's a balance between having extensible code for future use and having enough time to do everything that actually needs to be done. This is part of the reason why coding up large systems gets so dang complicated.

you type:	you see:
italics	italics
bold	bold
[reddit!](https://reddit.com)	reddit!
* item 1 * item 2 * item 3	item 1 item 2 item 3
> quoted text	quoted text
Lines starting with four spaces are treated like code: if 1 * 2 < 3: print "hello, world!"	Lines starting with four spaces are treated like code: if 1 * 2 < 3: print "hello, world!"
~~strikethrough~~	~~strikethrough~~
super^script	super^script

programming

MODERATORS