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 »

Volguus: Mar 3, 2009

Cuntpunch posted:

This is a lovely little gem.

code:

public static IEnumerable EnumerateUntilException(this IEnumerable list, Action<Exception> errorHandler)
{
	if (list == null) {yield break;}
	
	IEnumerator enumerator = null;
	try
	{
		enumerator = list.GetEnumerator();
		
		bool validItem;
		do
		{
			validItem = false;
			object item = null;
			try
			{
				if (enumerator.MoveNext())
				{
					item = enumerator.Current;
					validItem = true;
				}
			}
			catch (Exception ex)
			{
				if (errorHandler != null) {errorHandler(ex);}
			}
			
			if(validItem)
			{
				yield return item;
			}
		} while (validItem);
	}
	finally
	{
		var disposable = enumerator as IDisposable;
		if(disposable != null)
		{
			disposable.Dispose();
		}
	}
}

Oh, I see you're paying your developers per line of code written. Smart.

# ? Sep 4, 2019 13:38

Adbot: ADBOT LOVES YOU

# ? Jun 3, 2024 02:28

SupSuper: Apr 8, 2009; At the Heart of the city is an Alien horror, so vile and so powerful that not even death can claim it.

Cuntpunch posted:

This is a lovely little gem.

Is this the enterprise version of a try-catch?

# ? Sep 5, 2019 03:52

Remulak: Jun 8, 2001; I can't count to four.; Yams Fan

Yield keyword?

I�ve been doing embedded either too long or not long enough.

# ? Sep 5, 2019 04:29

Adhemar: Jan 21, 2004; Kellner, da ist ein scheussliches Biest in meiner Suppe.

Cuntpunch posted:

This is a lovely little gem.

code:

public static IEnumerable EnumerateUntilException(this IEnumerable list, Action<Exception> errorHandler)
{
	if (list == null) {yield break;}
	
	IEnumerator enumerator = null;
	try
	{
		enumerator = list.GetEnumerator();
		
		bool validItem;
		do
		{
			validItem = false;
			object item = null;
			try
			{
				if (enumerator.MoveNext())
				{
					item = enumerator.Current;
					validItem = true;
				}
			}
			catch (Exception ex)
			{
				if (errorHandler != null) {errorHandler(ex);}
			}
			
			if(validItem)
			{
				yield return item;
			}
		} while (validItem);
	}
	finally
	{
		var disposable = enumerator as IDisposable;
		if(disposable != null)
		{
			disposable.Dispose();
		}
	}
}

DefensiveProgramming.cs

# ? Sep 5, 2019 08:07

Joda: Apr 24, 2010; When I'm off, I just like to really let go and have fun, y'know?; Fun Shoe

Remulak posted:

Yield keyword?

I’ve been doing embedded either too long or not long enough.

Yield return for functions that return enumerables is a really nice little quality of life feature and I wish more languages had it.

In case you're wondering what it does, it allows the function to continue after the return statement and adds the returned value to the the enumerable ultimately returned by the function.

It has other nice features like if you enumerate it in a for loop it will only run as long as it needs to.

Joda fucked around with this message at 08:51 on Sep 5, 2019

# ? Sep 5, 2019 08:44

Beef: Jul 26, 2004

Hammerite posted:

It's considered harmful, Beef.

but actually I am mystified as to how you feel that code might be improved specifically via the use of goto.

It would definitely improve upon the parody of control flow that it is.

# ? Sep 5, 2019 08:48

moostaffa: Apr 2, 2008; People always ask me about Toad, It's fantastic. Let me tell you about Toad. I do very well with Toad. I love Toad. No one loves Toad more than me, BELIEVE ME. Toad loves me. I have the best Toad.

I found a couple of gems in one of our repos

code:

        Boolean qa = null;
        Boolean live = null;
        Boolean complete = null;
        Boolean active = true;
        if (environment == Environment.QA) {
            qa = true;
        } else if (environment == Environment.LIVE) {
            live = true;
        } else {
            qa = true;
        }

code:

	@Test
	public void testConvertListToArray_whenNullList_shouldReturnEmptyArray() {
		int[] array = convertListToArray(null);
		Assert.assertEquals(array.length, 0);
	}
	private int[] convertListToArray(List<Integer> tutorialIdsList) {
		if (tutorialIdsList == null) {
			return new int[0];
		}
		return ArrayUtils.toPrimitive(tutorialIdsList.toArray(new Integer[tutorialIdsList.size()]));
	}

# ? Sep 5, 2019 12:50

Munkeymon: Aug 14, 2003; Motherfucker's got an
armor-piercing crowbar! Rigoddamndicu𝜆ous.

Cuntpunch posted:

This is a lovely little gem.

[snip]

Hey, that's how Python stops enumeration :shrug:

more like

actually

Honestly the thing that annoys me most about this is that the author seems to be dimly aware of generics but went ahead and erased the type in the sequence anyway.

Beef posted:

It would definitely improve upon the parody of control flow that it is.

I don't see how it would?

# ? Sep 5, 2019 15:32

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

The better part of it is that it is only called in one place, where the errorhandler simply takes the returned Exception and throws a new exception by concatting it's message with some useless wrapper text (and erasing the callstack in the process), and that - well - this appears to be because some sort of Windows WMI Query enumerable occasionally throws? I don't know for sure, but I suspect all this rigamarole is to avoid an easily first-chance caught "Oh poo poo, we sent an invalid query to something that returns an enum and didn't check normal failure-reporting solutions."

# ? Sep 5, 2019 16:08

Suspicious Dish: Sep 24, 2011; 2020 is the year of linux on the desktop, bro; Fun Shoe

i wouldnt be surprised if thats copied from some stack overflow answer to doing some basic iterator task correctly because some weird edge case means you have to do it that way

# ? Sep 5, 2019 17:55

raminasi: Jan 25, 2005; a last drink with no ice

Suspicious Dish posted:

i wouldnt be surprised if thats copied from some stack overflow answer to doing some basic iterator task correctly because some weird edge case means you have to do it that way

I don't even think you need Stack Overflow. That's not far from what you'd come up with if you started out trying to write a normal-ish iterator with that error handling behavior (which is kind of unusual but not completely insane) and ran into the fact that you can't yield return inside a catching try.

# ? Sep 6, 2019 03:52

Happy Thread: Jul 10, 2005; by Fluffdaddy; Plaster Town Cop

Several pages ago some folks and myself were wondering if there were any "recommend me a password manager" threads and other general security / antivirus help. I just found this thread, which might be the closest thing I've seen to that, so here you go:

https://forums.somethingawful.com/showthread.php?threadid=3723583
Your Operating System has Poor Operational Security

# ? Sep 6, 2019 05:46

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

C#: nameof(OtherClass.Member) generates CS0122 (blah is inaccessible due to its protection level) if Member is declared private in OtherClass

# ? Sep 12, 2019 12:48

Volte: Oct 4, 2004; woosh woosh

Hammerite posted:

C#: nameof(OtherClass.Member) generates CS0122 (blah is inaccessible due to its protection level) if Member is declared private in OtherClass

What's wrong with that? Private members need to be able to be removed or renamed without breaking external code.

# ? Sep 12, 2019 13:05

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

Volte posted:

What's wrong with that? Private members need to be able to be removed or renamed without breaking external code.

If you're in a context where you can "see" that the private member exists (i.e. you're working in the same project) then I don't see why you can't refer to it by name in a way that just creates a const string...

Case in point, this happens even when OtherClass is internal and you're trying to use nameof() to refer to the private member from the same assembly.

code:

internal class A
{
    public void CleanUpStrings(List<string> strings)
        => RemoveButts(strings);

    private void RemoveButts(List<string> strings)
    {
        for (int i = 0; i < strings.Count; ++i)
        {
            if (strings[i] == "BUTT")
            {
                strings.RemoveAt(i);
                --i;
            }
        }
    }
}

public class B
{
    public void ProcessStrings(List<string> strings)
    {
        foreach (string s in strings)
        {
            // CS0122
            Debug.Assert(
                s != "BUTT",
                $"You were supposed to remove all the butts in {nameof(A)}.{nameof(A.RemoveButts)}(). You tool. You absolute buffoon"
            );
        }
    }
}

# ? Sep 12, 2019 13:18

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

I haven't thought through what the implications of changing this, if any, would be. I'm just having a whinge

# ? Sep 12, 2019 13:30

Volte: Oct 4, 2004; woosh woosh

Hammerite posted:

If you're in a context where you can "see" that the private member exists (i.e. you're working in the same project)

The issue is that "the same project" doesn't constitute a context where you can see that a private member exists. RemoveButts should be internal if that's the behaviour you want, since that's the whole point of internal visibility.

# ? Sep 12, 2019 13:31

Xik: Mar 10, 2011; Dinosaur Gum

Hammerite posted:

If you're in a context where you can "see" that the private member exists (i.e. you're working in the same project) then I don't see why you can't refer to it by name in a way that just creates a const string...
Case in point, this happens even when OtherClass is internal and you're trying to use nameof() to refer to the private member from the same assembly.

In your example the class is marked as internal, but RemoveButts is private. It sounds like you expect RemoveButts to behave as if it's internal, but if it did then there would be no difference between private and internal accessibility. The primary use case of private is when you're intentionally hiding that implementation and don't want external references.

Is this in Visual Studio? Did intilisense offer RemoveButts? Because that's the real coding horror, the fact the compiler complains isn't.

# ? Sep 14, 2019 00:25

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

Xik posted:

In your example the class is marked as internal, but RemoveButts is private. It sounds like you expect RemoveButts to behave as if it's internal, but if it did then there would be no difference between private and internal accessibility. The primary use case of private is when you're intentionally hiding that implementation and don't want external references.

Is this in Visual Studio? Did intilisense offer RemoveButts? Because that's the real coding horror, the fact the compiler complains isn't.

I don't want to be able to call the method from outside of the class, but it feels wrong to me that I can't merely refer to it in the context of nameof. As I said previously I'm making an armchair criticism here and haven't considered on any level what the consequences would be if it were different.

I realised in the shower, though, that a better workaround than just making the method public (or internal) would be to add an internally-visible string constant defined using nameof.

code:

internal class A
{
    public void CleanUpStrings(List<string> strings)
        => RemoveButts(strings);

    private void RemoveButts(List<string> strings)
    {...}

    internal const string MethodNameForAssert_RemoveButts = nameof(RemoveButts);
}

public class B
{
    public void ProcessStrings(List<string> strings)
    {
        foreach (string s in strings)
        {
            Debug.Assert(
                s != "BUTT",
                $"You were supposed to remove all the butts in {nameof(A)}.{A.MethodNameForAssert_RemoveButts}(). You tool. You absolute buffoon"
            );
        }
    }
}

# ? Sep 16, 2019 11:32

leper khan: Dec 28, 2010; Honest to god thinks Half Life 2 is a bad game. But at least he likes Monster Hunter.

Hammerite posted:

I don't want to be able to call the method from outside of the class, but it feels wrong to me that I can't merely refer to it in the context of nameof. As I said previously I'm making an armchair criticism here and haven't considered on any level what the consequences would be if it were different.

I realised in the shower, though, that a better workaround than just making the method public (or internal) would be to add an internally-visible string constant defined using nameof.
code:
internal class A
{
    public void CleanUpStrings(List<string> strings)
        => RemoveButts(strings);

    private void RemoveButts(List<string> strings)
    {...}

    internal const string MethodNameForAssert_RemoveButts = nameof(RemoveButts);
}

public class B
{
    public void ProcessStrings(List<string> strings)
    {
        foreach (string s in strings)
        {
            Debug.Assert(
                s != "BUTT",
                $"You were supposed to remove all the butts in {nameof(A)}.{A.MethodNameForAssert_RemoveButts}(). You tool. You absolute buffoon"
            );
        }
    }
}

Hth; maybe call out the public method not the private one. You can even use nameof still.

What do you want people to do, reflect to some private method?

# ? Sep 16, 2019 11:51

SirViver: Oct 22, 2008

^ Yeah, assuming your example is somewhat comparable to your real world case, I'd very much agree that you should reference "CleanUpStrings" in your assert message instead. The private implementation details of class A should not leak out of that class, even if you're trying to be helpful. Doing so just makes the code less maintainable, as you introduce weird and otherwise useless constants (i.e. clutter) to your classes and increase the amount of work required for refactoring. Sure, a simple rename would be picked up automatically (except your constant now refers to a non-existing "..._RemoveButts" method), but if you were to replace RemoveButts completely then you now suddenly have to also fix X other places accessing the constant, even though you didn't touch the "public interface" of the class. Or whoever edits the code don't care about/understand what the constant does, and now your asserts misleadingly refer to completely wrong methods. Those would then be just as "helpful" as comments that don't get updated to reflect what the code actually does.

# ? Sep 16, 2019 12:36

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

leper khan posted:

Hth; maybe call out the public method not the private one. You can even use nameof still.

SirViver posted:

^ Yeah, assuming your example is somewhat comparable to your real world case, I'd very much agree that you should reference "CleanUpStrings" in your assert message instead. The private implementation details of class A should not leak out of that class, even if you're trying to be helpful.

I disagree that this would be desirable. I think it's quite reasonable for assert messages in a debug build to refer to implementation details, and to "try to be helpful". They should ideally communicate what the author of the code was thinking when they wrote it.

quote:

What do you want people to do, reflect to some private method?

I'm sorry, I can't respond to this question because it is bizarre to me. I am unable to reconstruct the reasoning that led you to ask me this.

quote:

Doing so just makes the code less maintainable, as you introduce weird and otherwise useless constants (i.e. clutter) to your classes

That is a pity, but sometimes workarounds must be implemented for defects in the language you are working in (in case it's not clear, the "defect" I refer to here is what I originally posted to complain about; the inability to refer to objects from a nameof expression).

quote:

... and increase the amount of work required for refactoring. Sure, a simple rename would be picked up automatically (except your constant now refers to a non-existing "..._RemoveButts" method), but if you were to replace RemoveButts completely then you now suddenly have to also fix X other places accessing the constant, even though you didn't touch the "public interface" of the class.

So be it - this doesn't matter.

quote:

Or whoever edits the code don't care about/understand what the constant does, and now your asserts misleadingly refer to completely wrong methods. Those would then be just as "helpful" as comments that don't get updated to reflect what the code actually does.

This on the other hand does matter, because comments and similar material in code rot as you observe, and I will consider its implications.

# ? Sep 16, 2019 16:00

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

Apparently Friday was simultaneously 2019 Day of the Programmer and World Sepsis Day. I thought this thread might get a kick out of that.

# ? Sep 16, 2019 18:31

Munkeymon: Aug 14, 2003; Motherfucker's got an
armor-piercing crowbar! Rigoddamndicu𝜆ous.

Hammerite posted:

Apparently Friday was simultaneously 2019 Day of the Programmer and World Sepsis Day. I thought this thread might get a kick out of that.

Yep, thanks for that. Do they co-occur every year?

# ? Sep 16, 2019 18:45

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

Munkeymon posted:

Yep, thanks for that. Do they co-occur every year?

according to this page World Sepsis Day is the 13th of September every year, and according to Wikipedia Day of the Programmer is the 256th day of every year. So no, not every year.

# ? Sep 16, 2019 18:51

Jethro: Jun 1, 2000; I was raised on the dairy, Bitch!

Hammerite posted:

[snip]

While I kinda see what you're getting at, having the visibility of nameof(Class.Member) be different than the visibility of just Class.Member seems like the sort of thing that could have plenty of unintended consequences and might be hard to codify in a straightforward to implement rule.

Whereas "private stuff is private" is a pretty easy rule to understand.

# ? Sep 16, 2019 19:20

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

At its most basic, it means that changing the name of a private member can suddenly affect code outside the class, which is a huge violation of encapsulation. It might not have any bad consequences in this specific case but it�s a bad path to go down because the whole point of private members is to avoid that kind of thing.

If other code needs to know anything about the member, it shouldn�t be private.

# ? Sep 16, 2019 19:31

Volte: Oct 4, 2004; woosh woosh

Hammerite posted:

quote:
... and increase the amount of work required for refactoring. Sure, a simple rename would be picked up automatically (except your constant now refers to a non-existing "..._RemoveButts" method), but if you were to replace RemoveButts completely then you now suddenly have to also fix X other places accessing the constant, even though you didn't touch the "public interface" of the class.
So be it - this doesn't matter.

It's basically the only thing that matters when it comes to visibility. The entire point of encapsulation is to put boundaries on what code is allowed to form a dependency on what other code - for any given piece of code, you can be sure that nothing outside of its stated dependency boundary (i.e. its visibility specifier) is depending on it directly. Private visibility means the class implementation is the boundary, and nothing outside that is allowed to form a dependency on that member. If changing a private member means that external code has to be refactored, then that boundary was violated and your "private" member wasn't really private at all. There's no discussion to be had here, really - private means private, or else it doesn't mean anything.

Your workaround is the "right" way to do the specific thing you were trying to do - it's really no different from a getter for a private field, so in that sense it's a totally normal encapsulated property. On the other hand, I don't think that making a new property just to facilitate one specific debug message in an unrelated class is the right tactic. If anything, I would add some more specific internal debugging methods to class 'A' so that it can generate the debug message itself. Hell, you could make a hyper-specific internal method on A called AbortDueToDeveloperNotRemovingTheButtsProperly() that throws an exception and calls out whatever private details you want in the error message, and then call that from your other class when necessary. The important points are:

(a) external classes cannot depend directly (by name) on your private members
(b) external classes can and will depend directly (by name) on whatever conduit you use to enable access to private details.

In particular, if you let the conduit in question be nameof(A.PrivateMember) then (a) and (b) contradict each other.

Volte fucked around with this message at 20:51 on Sep 16, 2019

# ? Sep 16, 2019 20:42

Kuule hain nussivan: Nov 27, 2008

More fun times with our lovely software provider. I sent them a change request regarding one of the tools they provide to us, because one process in it was inconsistent with the main program. Received an email saying that a fix would be complex and costly, so they won't be changing anything right now, but will consider it in the future when they do a larger maintenance phase (gently caress if I know what this means).

Anyway, since this is the one part of their whole system where the source code isn't closed to us, I decided to have a stab at it myself, despite not having any documentation or previous knowledge of the code base. 5 minutes later, the change has been implemented. gently caress those assholes.

# ? Sep 19, 2019 20:59

Dirt Road Junglist: Oct 8, 2010; We will be cruel
And through our cruelty
They will know who we are

Kuule hain nussivan posted:

Received an email saying that a fix would be complex and costly, so they won't be changing anything right now, but will consider it in the future when they do a larger maintenance phase (gently caress if I know what this means).

That's generally code for, "We aren't paying attention to you and don't wanna deal with your complaints, so we're gonna pretend it's gonna cost YOU money so you'll hesitate and maybe even stop asking for it."

# ? Sep 19, 2019 21:05

Happy Thread: Jul 10, 2005; by Fluffdaddy; Plaster Town Cop

Lol that must be someone from the other side of the exchange from last page

duz posted:

"Sorry no, our system can't handle that type of change."

# ? Sep 20, 2019 01:51

Gibbon: Feb 22, 2004; chang chang!

PHP code:


if (md5(str1) == md5(str2)) {

Some spicy php I found a while back written by a contractor

# ? Sep 23, 2019 13:49

Ola: Jul 19, 2004

Gibbon posted:

PHP code:
if (md5(str1) == md5(str2)) {
Some spicy php I found a while back written by a contractor

Insecure. He should be doing 1024 iterations of SCrypt, then it's simply a matter of comparing the hash values afterwa...hmmm.

# ? Sep 23, 2019 14:08

DaTroof: Nov 16, 2000; CC LIMERICK CONTEST GRAND CHAMPION
There once was a poster named Troof
Who was getting quite long in the toof

Gibbon posted:

PHP code:
if (md5(str1) == md5(str2)) {
Some spicy php I found a while back written by a contractor

Maybe he snuck in a back door that depends on a collision.

# ? Sep 23, 2019 14:29

Athas: Aug 6, 2007; fuck that joker

Gibbon posted:

PHP code:
if (md5(str1) == md5(str2)) {
Some spicy php I found a while back written by a contractor

This is totally a backdoor. PHP's stupid == operator has some wonderful interactions with MD5 hashes. For example, md5('240610708') == md5('QNKCDZO') is true.

# ? Sep 23, 2019 14:39

Volguus: Mar 3, 2009

Athas posted:

This is totally a backdoor. PHP's stupid == operator has some wonderful interactions with MD5 hashes. For example, md5('240610708') == md5('QNKCDZO') is true.

Come on, please elaborate. How does the PHP's == operator work? How "should" it work? Does it not do logical comparison? Even if it does pointer comparison of the strings how could md5('x') == md5('y')? What's going on here? I understand that md5 is old and easy to find collisions, but this is on another level.

# ? Sep 23, 2019 14:50

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

Iirc md5() returns a hex string, and == interpreters string operands as decimal numbers (silently discarding everything from the first non-digit character onwards) and then does a numerical comparison. Or some such.

Php is full of footguns like this.

# ? Sep 23, 2019 14:54

Jewel: May 2, 2009

Volguus posted:

Come on, please elaborate. How does the PHP's == operator work? How "should" it work? Does it not do logical comparison? Even if it does pointer comparison of the strings how could md5('x') == md5('y')? What's going on here? I understand that md5 is old and easy to find collisions, but this is on another level.

PHP is garbage and "0wordshere" == 0 is true because "The string has a number in it! It failed the exact equality check so lets try and parse it as a number instead"

the """answer""" is to use === so it doesn't do the fuzzy comparisons which are horrible every time but the actual answer is gently caress php

# ? Sep 23, 2019 14:55

Volguus: Mar 3, 2009

Ouch. That reminds me of another language with equally awesome behaviour: https://dorey.github.io/JavaScript-Equality-Table/

# ? Sep 23, 2019 15:04

Adbot: ADBOT LOVES YOU

# ? Jun 3, 2024 02:28

Dylan16807: May 12, 2010

In this case php's at least trying a little bit to make sure they're numbers. The strings need to be 0 with an exponent, like 0e456789. So about one in 300 million md5 hashes does this.

And while it's easier to find strings of that form, you can currently make yourself a new unique md5 collision with 50 cents of computing power.

Dylan16807 fucked around with this message at 15:33 on Sep 23, 2019

# ? Sep 23, 2019 15:29

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 »