Coding Horrors: You can gather all your technical debt into one easy framework!

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›1503 »

RPATDO_LAMD: Mar 22, 2013; 🐘🪠🍆

Qwertycoatl posted:

I wonder what maintenance problems you get with a giant struct that you don't get wth 26 somewhat less giant structs grouped by name rather than purpose.

It looks like they did it this way so each struct can have its own magic-number sentinel value at the end, letting them check (at runtime) to see whether the struct definition and and its initializer list have gotten out of sync
i guess that's easier than figuring out which entry in a multi-hundred-line struct is missing but it still seems like it could be replaced by a compiler warning flag?

from decl.c:

C code:


#define MAGICCHECK(xx) \
    do {                                                                   \
        if ((xx).magic != IVMAGIC) {                                       \
            raw_printf(                                                    \
                 "decl_globals_init: %s.magic in unexpected state (%lx).", \
                       #xx, (xx).magic);                                   \
            exit(1);                                                       \
        }                                                                  \
        if ((xx).havestate != TRUE) {                                      \
            raw_printf(                                                    \
                 "decl_globals_init: %s.havestate not True.", #xx);        \
            exit(1);                                                       \
        }                                                                  \
    } while(0);

and then:

C code:

    MAGICCHECK(g_init_a);
    MAGICCHECK(g_init_b);
    MAGICCHECK(g_init_c);
[...]
    MAGICCHECK(g_init_x);
    MAGICCHECK(g_init_y);
    MAGICCHECK(g_init_z);

RPATDO_LAMD fucked around with this message at 20:43 on Jan 19, 2023

# ? Jan 19, 2023 20:37

Adbot: ADBOT LOVES YOU

# ? Jun 6, 2024 09:28

Falcon2001: Oct 10, 2004; Eat your hamburgers, Apollo.; Pillbug

QuarkJets posted:

lmao

Is your team checking coverage at all? You should be able to point to all of the code with 0% coverage and be like "hey you're not actually testing any of this".

The real coding horror is me, because as it turns out , this package is a wrapper for another client, (because we layer on some extra functionality to these API calls), so in FACT, it looked like this:

Python code:


def test_get_widget():
	expected_response = ["OUTCOME"]
	class_to_test.client.get_widget = Mock(response=expected_response)
	assert class_to_test.get_widget(test_parameters) == expected_response

not the mock being assigned to the client attribute, not directly to the API.

I discovered this, of course, after totally rewriting our test suite in PyTest and moving all the tests over and rewriting them. So...wasted a few hours, but not that big of a deal, and things are nicer now.

I think I might update that attribute to have an underscore so it's easier to notice, since it's not intended to be exposed. It's almost like 'private' variable nomenclature works sometimes.

# ? Jan 20, 2023 02:28

Jabor: Jul 16, 2010; #1 Loser at SpaceChem

Sorry but those tests are still dumb as hell.

A method that simply delegates to a different method with no actual logic of its own does not need to be tested. If the method does contain logic, then the tests should actually exercise that logic.

# ? Jan 20, 2023 02:42

dougdrums: Feb 25, 2005; CLIENT REQUESTED ELECTRONIC FUNDING RECEIPT (FUNDS NOW)

This seems like the place for this email I got, despite it not being strictly coding:

quote:

Dear Franchise Fans,

Our goal today is to provide more information on the Franchise Cloud Save issue from 12/28 and a timeline of next steps.

Status: On 12/30, we provided an update around Franchise cloud save data where some leagues became corrupted during a 10 hour window on December 28th. Our teams quickly began investigating and working to recover leagues. We've finished our root cause analysis and determined the issue impacted roughly 2% of the total number of existing leagues. However, for those leagues who accessed during the incident window, the majority resulted in an unrecoverable state.

Today, the team restored a limited number of Franchise saves via a backup. Unfortunately, those are all the available leagues that can be restored. For players looking to restart their league, a Week 17 �Play Now� Live roster went live last week with another Wild Card update available now as a starting point.

For more information on the issue and next steps, here is Madden NFL Executive Producer Seann Graddy:

Most importantly, we want to apologize for the issue. We know how important your Franchises are and feel your frustration around this issue. We deeply appreciate your loyalty to Madden and wanted to share some additional information and steps taken to ensure it never happens again.

The issue that occurred was the result of a highly unlikely set of combined circumstances leading to file server storage reaching max capacity, unutilized legacy code corrupting accessed leagues, while at the same time process errors led to almost all available backups getting deleted in an attempt to correct the original problem. Our team has done a full root cause analysis with technical experts to ensure infrastructure protocols, auto-scaling server systems, and legacy code are updated to prevent this uncommon event from ever occurring again.

Simply put: this should not have happened. While there is no making up for lost time, we would like to demonstrate our commitment to you.

unutilized legacy code, �process errors led to almost all available backups getting deleted in an attempt to correct the original problem,� like lmao what

dougdrums fucked around with this message at 04:18 on Jan 20, 2023

# ? Jan 20, 2023 03:50

Jabor: Jul 16, 2010; #1 Loser at SpaceChem

Given that the initial cause is mentioned as "file server storage reaching max capacity", a playbook entry of "file server is full -> delete older backups to free up space" is pretty easy to imagine.

# ? Jan 20, 2023 03:59

dougdrums: Feb 25, 2005; CLIENT REQUESTED ELECTRONIC FUNDING RECEIPT (FUNDS NOW)

Ah that makes sense, but I can�t help but to imagine a few more steps in between there. I can kinda imagine being cheap enough that the answer isn�t throw more money at the cloud until the cause is figured out. Still scratching my head at �unutilized legacy code� though.

# ? Jan 20, 2023 04:05

Ranzear: Jul 25, 2013

Carbon dioxide posted:

https://twitter.com/JeroenFrijters/status/1615204074588180481

https://twitter.com/steveklabnik/status/1616157231421980694

# ? Jan 21, 2023 00:14

ultrafilter: Aug 23, 2007; It's okay if you have any questions.

https://twitter.com/i2talics/status/1615465332382081024

# ? Jan 21, 2023 00:18

redleader: Aug 18, 2005; Engage according to operational parameters

i always thought the original was Fine, but the more alternatives i see the more convinced i become that it's not merely Fine, but actually superior

# ? Jan 21, 2023 13:09

LongSack: Jan 17, 2003

I would use a switch expression:

C# code:

return percentage switch {
  0.0 => ���,
  > 0.0 and <= 1.0 => ���,
  > 1.0 and <= 2.0 => ���,
  �
  _ => ���
}

# ? Jan 22, 2023 00:17

RPATDO_LAMD: Mar 22, 2013; 🐘🪠🍆

LongSack posted:

I would use a switch expression:

C# code:

return percentage switch {
  0.0 => ���,
  > 0.0 and <= 1.0 => ���,
  > 1.0 and <= 2.0 => ���,
  �
  _ => ���
}

That's identical to the original dutch-government thing that set this discussion off

# ? Jan 22, 2023 00:20

mmkay: Oct 21, 2010

https://twitter.com/TyrannoSaurav/status/1615492525997035521

Yeah, you should try to optimize it this way instead.

# ? Jan 22, 2023 06:39

brap: Aug 23, 2004; Grimey Drawer

RPATDO_LAMD posted:

That's identical to the original dutch-government thing that set this discussion off

fwiw the compiler actually eliminates the unnecessary comparisons in this case.

# ? Jan 22, 2023 10:00

Kazinsal: Dec 13, 2011

ultrafilter posted:

https://twitter.com/i2talics/status/1615465332382081024

This was more or less the solution I came up with as well and it's probably a sign I've been in kernel space for too long.

The other was slightly more lines but involved constructing the string one codepoint a time on the stack to save memory.

# ? Jan 22, 2023 10:14

Soricidus: Oct 21, 2010; freedom-hating statist shill

brap posted:

fwiw the compiler actually eliminates the unnecessary comparisons in this case.

Does it?

Like, it could in this case, but I�d be kind of surprised if they bothered to implement that, given that it�s only possible in certain specific situations and the benefit is likely to be insignificant unless there are vast numbers of cases.

# ? Jan 22, 2023 18:27

rjmccall: Sep 7, 2007; no worries friend; Fun Shoe

It falls out of any basic CSE / GVN optimization to the point that it would be weird not to do it. And eliminating redundant branches is a very worthwhile optimization in general; people tend to write a lot of redundant range checks, and reducing branch density is very good on modern processors.

# ? Jan 22, 2023 18:39

Dijkstracula: Mar 18, 2003; You can't spell 'vector field' without me, Professor!

rjmccall posted:

It falls out of any basic CSE / GVN optimization to the point that it would be weird not to do it.

this is all correct of course - though, the original method was written in Java and I've had cases where I wanted something along the lines of value numbering to be applied, but only the server compiler actually did it when I inspected the assembly. (maybe getPercentageRounds() is called often enough that it trips the recompilation threshold though :v:

)

# ? Jan 22, 2023 18:49

OddObserver: Apr 3, 2009

rjmccall posted:

It falls out of any basic CSE / GVN optimization to the point that it would be weird not to do it. And eliminating redundant branches is a very worthwhile optimization in general; people tend to write a lot of redundant range checks, and reducing branch density is very good on modern processors.

... How does NaN handling work out for that? I guess it follows from reasoning like:
(percentage <= 0.2) implies percentage is not NaN

!(percentage <= 0.1) and percentage not NaN
implies percentage > 0.1 ?

...which seems annoying, but I guess you gotta do it for this stuff to be useful for FP.
(And the evil --ffast-math from gcc might elide needing to prove !NaN?)

# ? Jan 22, 2023 18:51

rjmccall: Sep 7, 2007; no worries friend; Fun Shoe

Dijkstracula posted:

this is all correct of course - though, the original method was written in Java and I've had cases where I wanted something along the lines of value numbering to be applied, but only the server compiler actually did it when I inspected the assembly. (maybe getPercentageRounds() is called often enough that it trips the recompilation threshold though )

Yeah, in Java you definitely get some weird threshold effects because there�s do much abstraction that�s relatively difficult to break through � to see through a trivial getter, the JIT has to devirtualize and inline, and devirtualization in particular is usually a speculative optimization which the client JIT is tuned against doing. final helps a lot.

OddObserver posted:

... How does NaN handling work out for that? I guess it follows from reasoning like:
(percentage <= 0.2) implies percentage is not NaN

!(percentage <= 0.1) and percentage not NaN
implies percentage > 0.1 ?

...which seems annoying, but I guess you gotta do it for this stuff to be useful for FP.
(And the evil --ffast-math from gcc might elide needing to prove !NaN?)

In general, the biggest problem with optimizing FP is definitely that a lot of things which seem obvious mathematically turn out to be contingent or outright wrong because floating-point math is weird. So floating point comparison is only a partial order because of NaNs, which means we have transitivity between ordered values (thus e.g. knowing x < 1.0 and 1.0 < 2.0 implies x < 2.0), but negated transitivity is contingent on knowing that the values are ordered. But yeah, you have to teach the optimizer this stuff to do anything with FP at all, and FP code tends to be hot and very usefully optimizable.

# ? Jan 22, 2023 19:19

Dijkstracula: Mar 18, 2003; You can't spell 'vector field' without me, Professor!

rjmccall posted:

final helps a lot.

mad that it never occurred to me to mark all those temporaries as final

# ? Jan 22, 2023 19:28

Volte: Oct 4, 2004; woosh woosh

Dijkstracula posted:

this is all correct of course - though, the original method was written in Java and I've had cases where I wanted something along the lines of value numbering to be applied, but only the server compiler actually did it when I inspected the assembly. (maybe getPercentageRounds() is called often enough that it trips the recompilation threshold though )

It clearly specifies that the original method was written in C# though

# ? Jan 22, 2023 19:36

rjmccall: Sep 7, 2007; no worries friend; Fun Shoe

Dijkstracula posted:

mad that it never occurred to me to mark all those temporaries as final

If they�re locals, it shouldn�t make a difference, that�s not an abstraction the optimizer needs help with. That�s just JIT tuning working against you.

# ? Jan 22, 2023 19:56

Dijkstracula: Mar 18, 2003; You can't spell 'vector field' without me, Professor!

Volte posted:

It clearly specifies that the original method was written in C# though

I'm not exactly what you'd call a "reader of tweets" (oops, fair cop)

rjmccall posted:

If they�re locals, it shouldn�t make a difference, that�s not an abstraction the optimizer needs help with. That�s just JIT tuning working against you.

Interesting, I thought maybe C1 would know to fold those temporaries in (or otherwise look at them more closely) but I guess if you're not building up the use-def (or def-use? thanks to C2 I never know which is the standard one) then maybe there's just no hope for it, as you say

Dijkstracula fucked around with this message at 20:05 on Jan 22, 2023

# ? Jan 22, 2023 20:02

Vanadium: Jan 8, 2005

I think you're supposed to make the methods final, not the locals.

# ? Jan 22, 2023 20:26

Dijkstracula: Mar 18, 2003; You can't spell 'vector field' without me, Professor!

yeah, for monomorphizing + inlining I could see that being true (for the situation I was thinking of it wouldn't, but it's certainly possible that I way overfit the thing I was thinking about to the discussion at hand :v:

)

# ? Jan 22, 2023 20:32

Soricidus: Oct 21, 2010; freedom-hating statist shill

rjmccall posted:

It falls out of any basic CSE / GVN optimization to the point that it would be weird not to do it. And eliminating redundant branches is a very worthwhile optimization in general; people tend to write a lot of redundant range checks, and reducing branch density is very good on modern processors.

That�s happening at a lower level surely? I was talking about specifically the difference between the switch vs if/else chain, where I�d be surprised if the compiler emitted significantly different code for each.

# ? Jan 23, 2023 11:30

Wipfmetz: Oct 12, 2007; Sitzen ein oder mehrere Wipfe in einer Lore, so kann man sie ueber den Rand der Lore hinausschauen sehen.

Ranzear posted:

https://twitter.com/steveklabnik/status/1616157231421980694

The actual bug (as in: the program returning a different thing than expected) is the length, right? The returned strings are of different length for different percentages, and _all_ results include the zeroes at the end.

... or is this me realizing that I should think about NAN more often..? That NAN-discussion earlier made me nervous.

Wipfmetz fucked around with this message at 12:51 on Jan 24, 2023

# ? Jan 24, 2023 11:21

Xarn: Jun 26, 2015

both, but if you know that the output string is X chars long, you can just treat the string as such... of course every variant I've seen so far handles NaN badly, because remembering NaNs sucks.

# ? Jan 24, 2023 15:34

Wipfmetz: Oct 12, 2007; Sitzen ein oder mehrere Wipfe in einer Lore, so kann man sie ueber den Rand der Lore hinausschauen sehen.

I'd argue that having to know the intentioned length of a const char * - string would be a bug in itself, though.
But ok, maybe it was at least documented and that screenshot cut off said documentation.

Wipfmetz fucked around with this message at 10:19 on Jan 25, 2023

# ? Jan 25, 2023 07:21

cheetah7071: Oct 20, 2010; honk honk; College Slice

https://rdrr.io/cran/spatialEco/src/R/hli.R

quote:

tmp1 <- terra::terrain(x, v="slope", unit="degrees") * 0.017453293
tmp2 <- terra::terrain(x, v="aspect", unit="degrees") * 0.017453293
if(hemisphere == "northern" | force.hemisphere[1] == "northern"){
message("Using folded aspect equation for Northern hemisphere")
# Folded Aspect Northern Hemisphere (180 - (Aspect � 225) )
# 180(deg)=3.141593(rad), 225=3.92699(rad)
tmp3 <- terra::app(tmp2, fun=function(x) { abs(3.141593 - abs(x - 3.926991)) } )
} else if(hemisphere == "southern" | force.hemisphere[1] == "southern") {
message("Using folded aspect equation for Southern hemisphere")
# Folded Aspect Southern Hemisphere ( 180 - ( Aspect � 315) )
# 180(deg)=3.141593(rad), 315=5.49779
tmp3 <- terra::app(tmp2, fun=function(x) { abs(3.141593 - abs(x - 5.497791)) } )
}
tmp4 <- terra::app(tmp1, fun = cos)
tmp5 <- terra::app(tmp1, fun = sin)
tmp6 <- terra::app(tmp3, fun = cos)
tmp7 <- terra::app(tmp3, fun = sin)

when you're on "tmp7" I think it's time to use slightly more descriptive names

# ? Jan 26, 2023 23:01

Xerophyte: Mar 17, 2008; This space intentionally left blank

Can't help but feel that you buried the lede by not including the indentation

R code:

hli <- function(x, check = TRUE, force.hemisphere = c("none", "southern", "northern")) {
  if (!inherits(x, "SpatRaster")) 
    stop(deparse(substitute(x)), " must be a terra SpatRaster class object") 
  if(is.na(sf::st_crs(terra::crs(x))))	
    stop("Projection must be defined")	
  if(check) {
	if(!sf::st_is_longlat(x)) {
	  e <- sf::st_as_sf(terra::as.polygons(terra::ext(x)))
	    sf::st_crs(e) <- sf::st_crs(terra::crs(x))
	      l <- sf::st_bbox(sf::st_transform(e, 4326))[2]
    } else {
      l <- terra::ext(x)[3] 
    }	  
  }  
  if(l < 0) hemisphere = "southern" else hemisphere = "northern"  
    l = abs(l) * 0.017453293	
      cl = cos(l)
        sl = sin(l)
      tmp1 <- terra::terrain(x, v="slope", unit="degrees") * 0.017453293
    tmp2 <- terra::terrain(x, v="aspect", unit="degrees") * 0.017453293
	if(hemisphere == "northern" | force.hemisphere[1] == "northern"){ 
	  message("Using folded aspect equation for Northern hemisphere") 	
	    # Folded Aspect Northern Hemisphere  (180 - (Aspect � 225) )
		#   180(deg)=3.141593(rad), 225=3.92699(rad)
        tmp3 <- terra::app(tmp2, fun=function(x) { abs(3.141593 - abs(x - 3.926991)) } ) 
	} else if(hemisphere == "southern" | force.hemisphere[1] == "southern") {
	  message("Using folded aspect equation for Southern hemisphere") 		
		# Folded Aspect Southern Hemisphere  ( 180 - ( Aspect � 315) )  
		#   180(deg)=3.141593(rad), 315=5.49779 
		tmp3 <- terra::app(tmp2, fun=function(x) { abs(3.141593 - abs(x - 5.497791)) } ) 
	}	
          tmp4 <- terra::app(tmp1, fun = cos)
        tmp5 <- terra::app(tmp1, fun = sin)
      tmp6 <- terra::app(tmp3, fun = cos)
    tmp7 <- terra::app(tmp3, fun = sin)
	h <- exp( -1.467 +  1.582 * cl * tmp4  - 1.5 * tmp6 * tmp5 * sl - 0.262 * 
             sl * tmp5  + 0.607 * tmp7 * tmp5)
	if(terra::global(h, "max", na.rm=TRUE)[,1] > 1){
	  h <- ( h / terra::global(h, "max", na.rm=TRUE)[,1] ) 
    }		  
  return( h )
 }

# ? Jan 26, 2023 23:32

cheetah7071: Oct 20, 2010; honk honk; College Slice

I tried but forgot SA strips out whitespace in quote blocks

# ? Jan 26, 2023 23:34

Doom Mathematic: Sep 2, 2008

Java code:

int prime = 987654321; /* Arbitrary number. */

# ? Feb 17, 2023 23:41

Volmarias: Dec 31, 2002; EMAIL... THE INTERNET... SEARCH ENGINES...

Doom Mathematic posted:

Java code:

int prime = 987654321; /* Arbitrary number. */

A pretty prime code snippet there imo

# ? Feb 18, 2023 01:43

Toshimo: Aug 23, 2012; He's outta line...

But he's right!

That number isn't even prime.

# ? Feb 18, 2023 03:40

OddObserver: Apr 3, 2009

Toshimo posted:

That number isn't even prime.

I am embarrassed I didn't notice the divisibility by 9 at a glance. It's rather guaranteed by the way the number was chosen.

# ? Feb 18, 2023 03:53

I would blow Dane Cook: Dec 26, 2008

# ? Feb 18, 2023 04:47

more falafel please: Feb 26, 2005; forums poster

OddObserver posted:

I am embarrassed I didn't notice the divisibility by 9 at a glance. It's rather guaranteed by the way the number was chosen.

it's divisible by 3 even

without any context I'm gonna assume it's a sentinel value and whatever it's initialized to is arbitrary but that's maybe giving too much credit

# ? Feb 18, 2023 07:38

Hammerite: Mar 9, 2007; And you don't remember what I said here, either, but it was pompous and stupid.; Jade Ear Joe