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 »

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

notepad++ is real good, and feels way better (i.e. more responsive and smoother) than vs code which is, of course, electron garbage

# ? Apr 17, 2020 09:33

Adbot: ADBOT LOVES YOU

# ? May 21, 2024 07:22

Tempora Mutantur: Feb 22, 2005

i have an external hire senior engineer ("10+ years of experience!") who has had to be told on multiple occasions do not mock the class under test

we're talking

code:

@Mock private PojoService pojoServiceToTest

why'd he do it this time? because the tests weren't passing after the changes he made :suicide:

# ? Apr 17, 2020 09:42

SAVE-LISP-AND-DIE: Nov 4, 2010

Tempora Mutantur posted:

i have an external hire senior engineer ("10+ years of experience!") who has had to be told on multiple occasions do not mock the class under test

we're talking
code:
@Mock private PojoService pojoServiceToTest
why'd he do it this time? because the tests weren't passing after the changes he made

I'm sorting out a test suite with a good few instances of [Fact(Skip = "TODO: Make this work")] annotations on the tests. All I can think is at least it's better than commenting out the tests...?

Obviously the context is they added a bunch of poo poo to the code that broke the tests, then instead of figuring out how to fix it just left it.

# ? Apr 17, 2020 09:47

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

Tempora Mutantur posted:

i have an external hire senior engineer ("10+ years of experience!") who has had to be told on multiple occasions do not mock the class under test

we're talking
code:
@Mock private PojoService pojoServiceToTest
why'd he do it this time? because the tests weren't passing after the changes he made

how is this person still employed after multiple instances of gross incompetence?

# ? Apr 17, 2020 09:52

DoctorTristan: Mar 11, 2006; I would look up into your lifeless eyes and wave, like this. Can you and your associates arrange that for me, Mr. Morden?

Soricidus posted:

how is this person still employed after multiple instances of gross incompetence?

sociopaths tend to do extremely well in life

# ? Apr 17, 2020 11:46

Nth Doctor: Sep 7, 2010; Darkrai used Dream Eater!
It's super effective!

Tempora Mutantur posted:

i have an external hire senior engineer ("10+ years of experience!") who has had to be told on multiple occasions do not mock the class under test

we're talking
code:
@Mock private PojoService pojoServiceToTest
why'd he do it this time? because the tests weren't passing after the changes he made

My first read of your sentence had me imagining they were mocking like:
Look at this little bitch class. Ooh a service. Just like a beta class to serve their betters.

# ? Apr 17, 2020 13:41

qsvui: Aug 23, 2003; some crazy thing

Soricidus posted:

how is this person still employed after multiple instances of gross incompetence?

meritocracy is a myth

# ? Apr 17, 2020 19:04

Cuntpunch: Oct 3, 2003; A monkey in a long line of kings

Tempora Mutantur posted:

i have an external hire senior engineer ("10+ years of experience!") who has had to be told on multiple occasions do not mock the class under test

we're talking
code:
@Mock private PojoService pojoServiceToTest
why'd he do it this time? because the tests weren't passing after the changes he made

I've worked in teams where multiple senior, 10-25 year, engineers effectively thought unit testing was 'copy paste the code-under-test and compare its result to the code-under-test'

code:

public int RealAdd(int x, int y) => x + y

public int TestAdd(int x, int y) => x + y

public void RealAddTest() => Assert.Equals(RealAdd(2,2),TestAdd(2,2))

Imagine this but writ large.

# ? Apr 17, 2020 20:04

Tempora Mutantur: Feb 22, 2005

Cuntpunch posted:

Imagine this but writ large.

drat.

thank you all for commiserating, my sanity remains a little more intact. or at least the facade of it, whatever.

# ? Apr 17, 2020 22:24

elite_garbage_man: Apr 3, 2010; I THINK THAT "PRIMA DONNA" IS "PRE-MADONNA". I MAY BE ILLITERATE.

As a young impressionable junior dev, I will be adopting these new and unique testing strategies.

# ? Apr 18, 2020 01:48

Carbon dioxide: Oct 9, 2012

elite_garbage_man posted:

As a young impressionable junior dev, I will be adopting these new and unique testing strategies.

There's a better way, young padawan. When you're done with a feature just literally throw it over the cubicle wall into the testing department and consider it No Longer Your Problem. Make sure you build some physical defenses against them trying to throw stuff back.

# ? Apr 18, 2020 06:50

Absurd Alhazred: Mar 27, 2010; by Athanatos

Carbon dioxide posted:

There's a better way, young padawan. When you're done with a feature just literally throw it over the cubicle wall into the testing department and consider it No Longer Your Problem. Make sure you build some physical defenses against them trying to throw stuff back.

Trickle down featurenomics.

# ? Apr 18, 2020 06:55

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

elite_garbage_man posted:

As a young impressionable junior dev, I will be adopting these new and unique testing strategies.

my child, come pray with me at the Temple of the Works On My Machine

# ? Apr 18, 2020 07:38

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

Nth Doctor posted:

My first read of your sentence had me imagining they were mocking like:
Look at this little bitch class. Ooh a service. Just like a beta class to serve their betters.

Gotta admit, this one made me laugh. We could do with more of this mocking in this thread.

# ? Apr 20, 2020 21:09

VikingofRock: Aug 24, 2008

My old boss would use mocks to test the specific implementation of the method they were testing. So to test this class:

Java code:

public class Foo {
  private Bar bar;

  public Foo(Bar bar) {
    this.bar = bar;
  }

  public Thing doSomething() {
    int result_a = bar.method_a();
    String result_b = bar.method_b(result_a);
    return bar.method_c(result_b);
  }
}

they would write something like:

Java code:

public class FooTest {
  private static int A_RESULT = 123;
  private static String B_RESULT = "b result";
  private static Thing C_RESULT = new Thing();

  public void test_doSomething() {
    Bar bar = mock(bar);
    when(bar.method_a()).thenReturn(A_RESULT);
    when(bar.method_b(A_RESULT)).thenReturn(B_RESULT);
    when(bar.method_c(B_RESULT)).thenReturn(C_RESULT);
    
    Foo foo = new Foo(bar);
    foo.doSomething();
    
    verify(bar).method_a();
    verify(bar).method_b(ArgumentMatchers(A_RESULT));
    verify(bar).method_c(ArgumentMatchers(B_RESULT));
  }
}

Since the logic in Foo.doSomething() is so simple, this test trivially passes unless you mistyped something in the test itself. So all it does is make it harder to change the implementation of Foo.doSomething() in the future (if e.g. we realize we can avoid calling Bar.method_b() depending on the result of Bar.method_a() or whatever). I tried flagging this in code review a few times, but eventually just gave up because it wasn't worth arguing in the code review of almost literally every test they wrote.

# ? Apr 20, 2020 21:45

toiletbrush: May 17, 2010

I'm not familiar with Java so maybe I'm missing something but how else would you write a unit test for a method that is so dependant on a mocked service? I guess the first two 'verify's can be skipped but fix Foo and the test will be fine, it's not the test's fault.

If your point is more that tests like this do nothing but satisfy the Test Coverage Gods then I totally agree.

# ? Apr 21, 2020 20:13

VikingofRock: Aug 24, 2008

toiletbrush posted:

I'm not familiar with Java so maybe I'm missing something but how else would you write a unit test for a method that is so dependant on a mocked service? I guess the first two 'verify's can be skipped but fix Foo and the test will be fine, it's not the test's fault.

If your point is more that tests like this do nothing but satisfy the Test Coverage Gods then I totally agree.

My broader point is that this sort of test does nothing but satisfy the Test Coverage Gods. In this case specifically, I think a better test would be something like:

Java code:

public class FooTest {
  public void test_doSomething() {
    Bar bar = mock(bar);
    when(bar.method_a()).thenReturn(123);
    when(bar.method_b(123)).thenReturn("b result");
    when(bar.method_c("b result")).thenReturn(new Thing(););
    
    Foo foo = new Foo(bar);
    Thing result = foo.doSomething();

    // For the sake of this post, assume that these Things can be compared in this way
    assertThat(result).isEqualTo(new Thing());
    // alternatively, assertThat(result).hasSomeProperty();
  }
}

The reason is that you want to test the things that a user of Foo would care about. So you typically shouldn't test the specific service calls made by the method unless those are important to a users; instead you just want to test that your method returns the right things when the service acts in a certain way. So you mock up the service to whatever minimal level is necessary to get doSomething() to run, and then after that you treat doSomething() like a black box and just make assertions about its results. If the service is stateful, you probably want to set up your mock in such a way as to verify that the method leaves the service in the expected state, too.

That said, I have way less java experience than a lot of people in this thread, so it's always possible a java guru will pop in and tell us that I'm the real coding horror all along.

# ? Apr 22, 2020 02:36

Volguus: Mar 3, 2009

In my own personal opinion, I wouldn't test foo.doSomething() at all since, contrary to its name, it does nothing, except call Bar's methods. Then, let's test Bar's methods. Ensure that bar.method_a() and bar.method_b() and bar.method_c() do what they're supposed to do and return the correct result. No need to mock anything.
If foo.doSomething() later on changes and it actually does something, pull that algorithm in its own method, and test that with whatever params are needed. Again, no mocking required.
Now, if Bar is an external API that Foo is using, then no, we don't test that API. That's the responsibility of whoever wrote it to test it. Then mocking Bar and testing foo.doSomething() like VikingofRock suggested is the only way.
The idea being: avoid mocking if you can help it. Use it when you need to, but no more.

# ? Apr 22, 2020 03:46

Bruegels Fuckbooks: Sep 14, 2004; Now, listen - I know the two of you are very different from each other in a lot of ways, but you have to understand that as far as Grandpa's concerned, you're both pieces of shit! Yeah. I can prove it mathematically.

I just pull anything that looks complicated into static methods and just write tests for those and call it functional programming.

# ? Apr 22, 2020 05:23

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

VikingofRock posted:

My broader point is that this sort of test does nothing but satisfy the Test Coverage Gods. In this case specifically, I think a better test would be something like:
Java code:
public class FooTest {
  public void test_doSomething() {
    Bar bar = mock(bar);
    when(bar.method_a()).thenReturn(123);
    when(bar.method_b(123)).thenReturn("b result");
    when(bar.method_c("b result")).thenReturn(new Thing(););
    
    Foo foo = new Foo(bar);
    Thing result = foo.doSomething();

    // For the sake of this post, assume that these Things can be compared in this way
    assertThat(result).isEqualTo(new Thing());
    // alternatively, assertThat(result).hasSomeProperty();
  }
}
The reason is that you want to test the things that a user of Foo would care about. So you typically shouldn't test the specific service calls made by the method unless those are important to a users; instead you just want to test that your method returns the right things when the service acts in a certain way. So you mock up the service to whatever minimal level is necessary to get doSomething() to run, and then after that you treat doSomething() like a black box and just make assertions about its results. If the service is stateful, you probably want to set up your mock in such a way as to verify that the method leaves the service in the expected state, too.

That said, I have way less java experience than a lot of people in this thread, so it's always possible a java guru will pop in and tell us that I'm the real coding horror all along.

This is probably the right way to do it. You're testing that when you call a method with a certain input, given a certain state, that it will perform an expected action (return X value, throw Y exception, ensures Z is in the Frob state, although even that's iffy). If you're just checking that each element within the test is hit in a particular order, you're writing a change detector test that's only useful for failing when someone changes that code (that is, it's useless).

# ? Apr 22, 2020 05:51

NihilCredo: Jun 6, 2011; iram omni possibili modo preme:
plus una illa te diffamabit, quam multæ virtutes commendabunt

Bruegels Fuckbooks posted:

I just pull anything that looks complicated into static methods and just write tests for those and call it functional programming.

Yeah I do something similar whenever possible.

1) CRUD method(s) to take input objects from various sources
2) Pure method to process those inputs and product output objects
3) CRUD method(s) to save, print, or otherwise return the output

(2) gets unit tested. (1) and (3) can skip straight to integration or end-to-end tests.

# ? Apr 22, 2020 08:23

Carbon dioxide: Oct 9, 2012

Bruegels Fuckbooks posted:

I just pull anything that looks complicated into static methods and just write tests for those and call it functional programming.

I mean moving annoying I/O to the fringes of your program and having pure methods to test is one of the main benefits of functional progamming. You're not just calling it functional progamming, it IS functional programming.

# ? Apr 22, 2020 16:27

Aramis: Sep 22, 2009

Bruegels Fuckbooks posted:

I just pull anything that looks complicated into static methods and just write tests for those and call it functional programming.

You just gave me a flashback to something I ran into a little over 10 years ago while doing contract work on a codebase that had been maintained by a rotating team for very long time (a licensed sports video game). This included a C -> C++ transition that must have occurred in the early oughts.

code:

class Team {
public:
  static String get_name(int team_id);
  static void add_player(int team_id, Player* player);
  static Player* get_player(int team_id, int player_id);

  ...
};

Curious about the reasoning process that lead to this, I pulled up the commit history that happened to have miraculously survived the CSV->SVN->Perforce transition.

quote:

"Converted team management to be class-based."

Aramis fucked around with this message at 07:46 on Apr 23, 2020

# ? Apr 23, 2020 07:39

Spatial: Nov 15, 2007

Good old C+.

# ? Apr 23, 2020 11:30

xtal: Jan 9, 2011; by Fluffdaddy

Aramis posted:

You just gave me a flashback to something I ran into a little over 10 years ago while doing contract work on a codebase that had been maintained by a rotating team for very long time (a licensed sports video game). This included a C -> C++ transition that must have occurred in the early oughts.
code:
class Team {
public:
  static String get_name(int team_id);
  static void add_player(int team_id, Player* player);
  static Player* get_player(int team_id, int player_id);

  ...
};
Curious about the reasoning process that lead to this, I pulled up the commit history that happened to have miraculously survived the CSV->SVN->Perforce transition.

This reminds me of being 12 years old and my idea of OOP was writing a class that printed the header in the constructor and footer in the destructor and then having `new Blog()` at the end.

# ? Apr 23, 2020 15:06

Phobeste: Apr 9, 2006; never, like, count out Touchdown Tom, man

Anybody who intuitively gets RAII that early has a long dark life ahead

# ? Apr 23, 2020 15:16

Tei: Feb 19, 2011

Aramis posted:

Curious about the reasoning process that lead to this

Some form of "it must be in OOP form" and "it must finished by date". These are measurable, where the quality is not.

# ? Apr 24, 2020 09:51

DoctorTristan: Mar 11, 2006; I would look up into your lifeless eyes and wave, like this. Can you and your associates arrange that for me, Mr. Morden?

Did that class have any data members? Or any non-static methods at all?

# ? Apr 24, 2020 11:36

Aramis: Sep 22, 2009

Tei posted:

Some form of "it must be in OOP form" and "it must finished by date". These are measurable, where the quality is not.

This is measurably not OOP form, but it is measurably using a C++ feature that is named "class", so the commit message is "technically" correct while still being a complete lie.

DoctorTristan posted:

Did that class have any data members? Or any non-static methods at all?

Not a single one. All the programmer did was wrap the module in a class and slap static in front of all the functions. If that was the refactor their lead wanted, then they would have used a namespace. It was probably done out of lazyness, but I like to think that it was a curmudgeon programmer who did this out of malicious compliance. "Oh you want a class? Should have specified that you wanted OOP."

Aramis fucked around with this message at 15:17 on Apr 24, 2020

# ? Apr 24, 2020 15:00

Absurd Alhazred: Mar 27, 2010; by Athanatos

Aramis posted:

This is measurably not OOP form, but it is measurably using a C++ feature that is named "class", so the commit message is "technically" correct while still being a complete lie.

Not a single one. All the programmer did was wrap the module in a class and slap static in front of all the functions. If that was the refactor their lead wanted, then they would have used a namespace. It was probably done out of lazyness, but I like to think that it was a curmudgeon programmer who did this out of malicious compliance. "Oh you want a class? Should have specified that you wanted OOP."

Could be someone with Java training.

# ? Apr 24, 2020 15:31

Tei: Feb 19, 2011

Quake was pure C, and one of the first things Valve did with Half-Life was to convert the codebase to C++.

If you look at it (was leaked in a unfortunate accident) is very plain OOP code, don't do fancy things with objects, is a pretty literal translation.

Some files, like cl_client.cpp, are a almost direct translation from cl_client.c

imo:

Early. C to C++ conversion probably had people scared of automatic operations in memory. Like some comparison triggering a structure being copied in memory and doing other operations that are non-trivial for a C developer.

Once game developers where more acustomed to C++, they probably had less poo poo to give about these events, or maybe moved far from the metal so they did not knew these operations existed and their impact is not really big.

I am a idiot, so probably 104% of this is wrong, anyway. Just writting words here.

# ? Apr 24, 2020 16:48

Foxfire_: Nov 8, 2010

It does look OOP though? It's not using syntax to make life easier, but there are conceptual Team and Player objects owning data and operations (assuming there's more functions that were omitted), just accessed with an explicit this parameter instead of implicit.

(Assuming that is a index into a global table instead of wrangling with custom allocators)

It's not good syntax, but it's not a design horror at least.

# ? Apr 24, 2020 17:28

Absurd Alhazred: Mar 27, 2010; by Athanatos

Foxfire_ posted:

(assuming there's more functions that were omitted)

Why are you assuming the opposite of what the poster wrote?

DoctorTristan posted:

Did that class have any data members? Or any non-static methods at all?

Aramis posted:

Not a single one. All the programmer did was wrap the module in a class and slap static in front of all the functions. If that was the refactor their lead wanted, then they would have used a namespace. It was probably done out of lazyness, but I like to think that it was a curmudgeon programmer who did this out of malicious compliance. "Oh you want a class? Should have specified that you wanted OOP."

# ? Apr 24, 2020 17:32

Aramis: Sep 22, 2009

I think a lot of you are missing the point that there's a story being told here.

The code existed, was fine, but was a little on the old-school side because that interface obviously manages globally allocated resources. It's a perfectly fine C-style way of implementing things. However, at some point, someone said: "Man, having a Team object to manipulate would be better than this pile of freestanding functions full of contextual assumptions", and a task to refactor the Team functions into a Team class was filed.

Whoever was assigned to this changed literally nothing, nothing at all. Not touching the code would have done the exact same thing functionality-wise. The only thing this did was allow them to hit the close button on the issue and not be technically lying.

Aramis fucked around with this message at 18:35 on Apr 24, 2020

# ? Apr 24, 2020 18:26

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

Making the methods static class functions does have the effect of taking them out of the global namespace. It's not object-oriented, but it's not nothing either.

# ? Apr 24, 2020 18:34

Xik: Mar 10, 2011; Dinosaur Gum

Aramis posted:

The only thing this did was allow them to hit the close button on the issue and not be technically lying.

If your performance metrics involved "number of closed calls" then that's the point of the job.

# ? Apr 24, 2020 20:48

Foxfire_: Nov 8, 2010

Absurd Alhazred posted:

Why are you assuming the opposite of what the poster wrote?

I'm making assumptions about what's inside the ... in the Team class (presumably more static functions that manipulate Team objects)

OOP is a design pattern, not any particular syntax. You can do it in languages with no explicit syntax support for it or do it in C++ without using member functions. It's clunkier and more verbose, but it has the same organizational advantages/disadvantages.

The classic C version would be like:

code:

struct SomeClass
{
    int aPrivateMemberVariable;
};

void SomeClass_Constructor(SomeClass* this);
void SomeClass_AMemberFunction(SomeClass* this, int aParameter);
int SomeClass_GetFoo(const SomeClass* this);
void SomeClass_Destructor(SomeClass* this);

and then relying on callers to (1) not directly access the variables in the struct, (2) explicitly call constructor/destructor functions.

The posted code looks like basically that, except using indices into a global table instead of something typesafe (which is bad!). It sounds like the original change request was kind of dumb and whoever got stuck with it wanted to satisfy a manager and move on, which seems reasonable to me (especially since the alternative would be to fight C++ allocator syntax to still use the globally allocated tables)

# ? Apr 24, 2020 21:11

Plorkyeran: Mar 22, 2007; To Escape The Shackles Of The Old Forums, We Must Reject The Tribal Negativity He Endorsed

ultrafilter posted:

Making the methods static class functions does have the effect of taking them out of the global namespace. It's not object-oriented, but it's not nothing either.

If only c++ had a way to do that without misusing a different feature.

# ? Apr 24, 2020 21:11

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

Tei posted:

Early C to C++ conversion probably had people scared of automatic operations in memory. Like some comparison triggering a structure being copied in memory and doing other operations that are non-trivial for a C developer.

Once game developers where more acustomed to C++, they probably had less poo poo to give about these events, or maybe moved far from the metal so they did not knew these operations existed and their impact is not really big.

If performance is so important that memory row access times vs the CPU cache matter, you want to use your own memory management system instead of hoping that the built in memory system does what you would like, so that you can ensure that the memory you're seeking is where you expect.

# ? Apr 24, 2020 21:40

Adbot: ADBOT LOVES YOU

# ? May 21, 2024 07:22

qsvui: Aug 23, 2003; some crazy thing

Plorkyeran posted:

If only c++ had a way to do that without misusing a different feature.

eh, using a struct with static methods is fine. you don't have to deal with the possibility of using declarations, people re-opening the struct from a completely different area of the codebase and adding poo poo like you can with namespaces, and also sidesteps any adl shenanigans.

# ? Apr 25, 2020 01:52

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 »