Get Rid of That Code Smell – Primitive Obsession • Hacker News Board

Re: Get Rid of That Code Smell – Primitive Obsession

#21

Post by tzs » Tue, Jun 26, 2012, 11:23 AM UTC

Negitivefrags wrote: post #14

dasil003 wrote: post #12

Earlier quoted context omitted.

You're being obtuse. What's your use case for money as floats?

I think his point is that the OP got burned using floats for money. He thought the lesson was to use a custom class for money every time, but the actual lesson could have just been to use integers instead.

Actually, the point was don't blame your tools.

If you understand how floating point works, you can deal with the rounding and truncation errors, and get accurate results for monetary calculations.

Re: Get Rid of That Code Smell – Primitive Obsession

#22

Post by yummyfajitas » Tue, Jun 26, 2012, 11:27 AM UTC

CodeMage wrote: post #18

yummyfajitas wrote: post #16

Earlier quoted context omitted.

Realtime trading systems. If your system is slow, you lose money. If your trading system says you earned $1,523,374.54, but your accounting system says you only earned $1,523,374.26, you don't care much.

I'm genuinely curious here: why would using integers be slower than using floats? I thought floating point operations were always more expensive than handling ints.

For simple accounting, it wouldn't be slower. The issue is analytical code, e.g.:

    int price = getPrice(symbol);
    double signal = exp(...) * ((double)price);
    if (signal > threshold) {
        buy(symbol);
    }

The point is to avoid converting from int to double every single time you need to do a calculation.

Re: Get Rid of That Code Smell – Primitive Obsession

#23

Post by cyclinghacker » Tue, Jun 26, 2012, 12:14 PM UTC

cageface wrote: post #9

This is exactly the opposite of the advice recently given by Rich Hickey in his keynote at RailsConf 2012, where he strongly recommends using simple, transparent data structures without a lot of OO wrapping baggage. Personally I'm on the fence but when you look at how much boilerplate this Virtus example needs to get rid of the code "smell" you have to wonder if maybe he has a point.

I think it just comes down to differences between the OO approach vs the functional approach.

In OO code I try to create types to represent every concept in my code. It seems overkill at first but I have found in most cases it pays off with reduced duplication. It almost always removes the question of where to put static utilities. Lets say you have a postcode - by creating a Postcode type you can put postcode validation, postcode formatting, etc, directly on that type rather than floating in different parts of your codebase.

Additionally, in a statically typed language it adds type safety - so for example you can't pass a postcode into an address parameter.

Re: Get Rid of That Code Smell – Primitive Obsession

#24

Post by wglb » Wed, Jun 27, 2012, 2:18 AM UTC

dasil003 wrote: post #7

There's another code smell that every programming whiz kid produces at some point: over-engineered. All code has a cost, primitives have a lower base cost because they are universal in the language and thus every programmer will automatically know how to use them. Before introducing a custom object with yet another API to be learned, you need justification. > Implement Money class if you need to deal with money, it’s…

Now that is in interesting proposition: Let's pretend he said integer since floats for money is outright broken. On my 32 bit machine, enquire.c tells me that long integers have 32 bits of unsigned value. It also says that doubles have 53 bits of precision. So if you decide to use floats for money, you get bigger numbers than if you use long.

Oh, and might I suggest that you consider doing it in pennies rather than fractional dollars? It will work out better.

Better to understand the underlying territory, I think. That would not be over-engineering.

Re: Get Rid of That Code Smell – Primitive Obsession

#25

Post by awongh » Thu, Jun 28, 2012, 8:24 AM UTC

I understand about making code more clear, but I'm not sure adding complexity/boilerplate solves that in every case- the even more extreme case would be to do away with the class entirely and just have a hash type property that gets passed around.... his very verbose counter-argument is the other extreme, and while I see his point, I tend to want to start with the property and only move towards the "rich object" when I'm sure that it's the best solution....

Re: Get Rid of That Code Smell – Primitive Obsession

#26

Post by solnic » Thu, Jun 28, 2012, 2:44 PM UTC

awongh wrote: post #25

I understand about making code more clear, but I'm not sure adding complexity/boilerplate solves that in every case- the even more extreme case would be to do away with the class entirely and just have a hash type property that gets passed around.... his very verbose counter-argument is the other extreme, and while I see his point, I tend to want to start with the property and only move towards the "rich object" when…

I think people miss the fact that I wrote about a code smell, which means a potential problem in your code. I don't say you shouldn't be using primitives. I say that there are cases where you should use rich, custom objects, that will simplify your code and improve design.

Re: Get Rid of That Code Smell – Primitive Obsession

#27

Post by awongh » Fri, Jun 29, 2012, 1:35 AM UTC

solnic wrote: post #26

awongh wrote: post #25

I understand about making code more clear, but I'm not sure adding complexity/boilerplate solves that in every case- the even more extreme case would be to do away with the class entirely and just have a hash type property that gets passed around.... his very verbose counter-argument is the other extreme, and while I see his point, I tend to want to start with the property and only move towards the "rich object" when…

I think people miss the fact that I wrote about a code smell, which means a potential problem in your code. I don't say you shouldn't be using primitives. I say that there are cases where you should use rich, custom objects, that will simplify your code and improve design.

absolutely there are those cases, maybe it was just an over simplified example.

Although I also wouldn't say that it simplifies your code... What it does is make the api to that class/object more explicit. At the very least you are keeping the complexity level the same, but you might also be making it more complicated. If you're just talking about the idea of OO encapsulation in general, that's another thing.... Not that I disagree that an explicit api is a good thing.