• Registered (unregistered)

    If (commentNum == 1) frist = true;

    A garbage collector stress test?

  • (nodebb)

    Apparently GetDataPoints doesn't clear the data buffer after returning the acquired data (pretty much all of the instruments that I've worked with do so). I feel like there might be some additional WTFs related to that somewhere else in the code.

  • TS (unregistered)

    No hate for tempDataPointArray? Let's give a variable an unwieldy name, most of which is telling us its type (what people criticise Systems Hungarian for, but with extra verbosity), and just four characters for an ambiguous abbreviation for its actual contents.

  • Jakoss (unregistered)

    You gotta keep the GC fit!

  • (nodebb) in reply to TS

    Poor naming of local variables is not the greatest sin in this code, not even close.

  • (nodebb)

    The fact that "GetDataPoints()" returns an object with a "Count" property indicates to me that it returns a "List'T". If it were returning an enumerable, as the post suggests, one would have to use the "Count()" function. It's possible it returns the actual contents of the backing "List'T" store, which would be mutable, so they make a defensive copy through "ToArray".

    However, as said, TRWTF is the enum conditions and preallocating the arrays just to overwrite them.

  • some guy (unregistered)

    Remy, I also had first the idea "why aren't they reusing the enum". But then I saw that one is sensorType and the other is sensorNum. Now, I assume that this is not a case of bad anonymization. However, it is a case of TRUE, FALSE, FILE_NOT_FOUND: they either have to use a switch with a defensive default branch or it should not be an enum at all.

  • (nodebb)

    That sensorType is being compared against anenum value (in the worst way) but sensorNum isn't... is actually fairly sane. Or at least potentially so; you'd need to know the actual hardware configuration to know for sure. Having just one humidity sensor but several temperature ones is a setup I can believe making good sense in some application.

  • (nodebb) in reply to dkf

    The worst bugs, implementations, type-coercion, and (absence of) structured code often start out with unclear naming conventions.

  • Sauron (unregistered)

    Remember that CodeSOD the next time the weather forecast promises a warm day of sunshine whereas the sky actually delivers a deluge and a wind so cold it chills your very bones.

  • (nodebb)

    I remember when dictionaries where invented to solve the whole enum index issue in arrays. Ah, those were the days, when the cold war was still going on and people had to remember numbers to call someone.

  • pudin9 (unregistered)

    I wouldn't be surprised if the empty array allocations would get thrown out by the compiler. In the latest versions, it can optimize code using astonishingly deep insights into the logic. Sure, it's still bad code but the GC is probably not involved.

  • (nodebb) in reply to MaxiTB

    Ah, those were the days, when the cold war was still going on and people had to remember numbers to call someone.

    What I remember of those days was people having a "little black book" to write those numbers down in. These days, that little black book is a slab of computing power that also plays music, makes phone calls, and so on.

  • DrPepper (unregistered)

    "and it's a best practice in C# to use more abstract interfaces for collections" -- except where I work, where everything is converted to a List. I think it was because at some point in time, there was a bug where a lazy-loaded list was evaluated and threw an exception, resulting in a bug. Instead of fixing the problem, a bandaid was applied -- everything is converted to a List, even if it is some other, relalized, enumerable type.

  • (nodebb)

    Are you sure SingleSensorTemptale is a misspelling? A series of data points from a temperature sensor tells the story of temperatures past; a temp-tale, if you will.

  • (nodebb) in reply to Doodpants

    Then it would still be an spelling error. SingleSensorTempTale would be the correct one.

  • (nodebb)

    It took me way too long to realise that if sensorType != HUMIDITY and sensorNum != 1, tempDataPointArray will return the same datapoints as for HUMIDITY.

    Up until that point, I was happy with the logical structure. The first if-else is to split up Humidity vs Temperature sensors. The second if says if it is a Temperature sensor we're after, which one?

    But yeah, I agree that this is likely either the work of a C programmer who has migrated to C#, or a new programmer that was trained by an old-school C programmer, or one that's been told to always initialise the size of your containers to what you need when you create them.

  • Jeremy (unregistered)

    It seems the primary humidity semsor was so unreliable that they chose to remove it from code.

    I guess a software engineer would not think of replacing the actual sensor

  • (nodebb)

    I love the smell of a sensor C sample code "ported" to C# by someone who either did not bother trying to understand why the C code was written this way (alloc array then fill), or doesn't understand C# references, or probably all of the above.

    And that's assuming the sensor C sample code was properly written to begin with, which isn't a given.

  • (nodebb)

    As I read it, the device has two sensors. The primary is always a temperature sensor, the secondary may be another temperature sensor or a humidity sensor. The logic doesn't seem the most obvious way to do it - the comment says "For now, determine if this is the primary or secondary sensor" but doesn't do that until later (or not at all). In fact, it first checks the sensor type. If it's a humidity sensor, it can only be the secondary, so process the secondary data into the humidity array. If it's a temperature sensor, we check if it's the primary. If it is, we put the primary data in the temperature array. If not, it must be the secondary temperature sensor, so we put that data in the temperature array. This logic will fall apart completely if the assumptions about the hardware aren't correct of course.

    And of course I'm assuming here that "temp" means temperature - I'm sure most programmers would read it as "temporary", as I did initially.

  • TheCPUWizard (unregistered)

    Be careful, the JIT optimizer is pretty smart, and can optimize object creation straight out of existence....

  • MaxiTB (unregistered) in reply to TheCPUWizard

    Yeah, but is is also very limited due to the arrays legacy nature. You can do a lot funky stuff with most people luckily are not aware of, so its always a good idea to avoid heap types.

    When it comes to arrays, working with spans and using ArrayPool is always a good idea, because the main reason why .net is now around twice as fast as the same Java applications is simply because they heavily removed heap allocations in the framework (ValueTask started it and every was actually super surprised how insane the gains after it was used in hot paths like streaming classes).

  • (nodebb) in reply to pudin9

    I would hope the allocation would be optimized out by the compiler. Otherwise there would seem to be a window between the allocation and the initialization for another sample to be taken and run past the end of the array.

Leave a comment on “Taking the Temperature”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article