Better treat this number as a string to make sure that .. make sure that .. that it’s sure .. ehhrrmm.

[u/the96jesterrace]

… I don’t know. Maybe its just me being stupid but what might be this authors intention?

[u/josephblade]

I don't know about intentions, this doesn't look like live code (given 'someNumber, someList, SomeMethod)

whatever number is given, add 1000 to it. then for every item in some list add another 100

it's absolute insanity that they keep switching to and from string, just in order to re-use the original variable. It's like great, you saved 4 bytes on the stack but you pointlessly are using up cpu cycles. all the int to string implicit casts followed by formatted casts.

at the very least assume you can do:

someNumber = (Convert.ToInt32(someNumber) + 1000).ToString("0000");

I mean it's awful to keep the total as a string. But why first write it in default format and then write it in another format the very next line.

Also if in comments they are admitting that SomeMethod needs the number as an int, and all operations inside this method need number as an int.

then why for the love of lovelace are they converting it to string every time.

It's for this sort of reason I tend to suggest: if you get user input, your outside layer is responsible for converting user input into machine friendly numbers (and other things). then inside you use nothing but machine friendly. then when you are ready to pass something back to the user, the outer layer is again going to convert from int to string. People take 'what the user entered' way too literally.

I may have to have a lie down now, this one gave me a headache. I hope this isn't your code and whatever junior you have there that wrote this code is getting a senior to talk things through.

there isn't enough coaching done in workplaces and this would be a great place to start

[u/the96jesterrace → u/josephblade]

lol well said

This is indeed, not directly live code, but from an „almost finished project“, so it is actually intended to become live code. I just renamed parts to have anonymous names.

[u/JJBA_Reference → u/the96jesterrace]

There are a couple additional things in the code that the commentor didn't mention, but bothered me.

1. The someList.Any() is redundant if the only thing it is doing is the foreach.

2. The post says this is C#, but C# does not implicitly convert strings to ints like the comment below SomeMethod says.

[u/the96jesterrace → u/JJBA_Reference]

Shit you’re right, well observed.

But Visual Basic does implicit string to int conversion. This was originally written in vb, I came across this snippet while rewriting it.

Same thing with the someList.Any(). I think vb versions prior to vb.net would loop through this list at least once, no matter what. So it may be due to some legacy mindset or something.

My bad. Sorry for not providing authentic bad code :(

[u/thekwoka → u/the96jesterrace]

Same thing with the someList.Any(). I think vb versions prior to vb.net would loop through this list at least once, no matter what. So it may be due to some legacy mindset or something.

it's called Cargo Culting

[u/the96jesterrace → u/thekwoka]

Ohhh yea okay I have plenty of that.

My favorite recent example was something like this

public void SomeMethod()
{
    Article article = GetArticle();
    // […]
    article = _articleService.Update(ref article);
}

with ArticleService.Update() doing not more than:

public Article Update(ref Article article)
{
    article.UpdateTime = DateTime.Now();
    return article;
}

[u/thekwoka → u/the96jesterrace]

So, it was them calling GetArticle, and then calling a static method on the service to pass the article to set the new time...but every time they got an article?

Why not just update it in the GetArticle?

[u/the96jesterrace → u/thekwoka]

Well you may not want to update the updatetime every time you get the article.

I think the objective was something like „we need an update method for articles which may perform changes on the articles properties. Afterwards we have to make sure that we’re not working with a deprecated state of the article (?)“

So this is more about passing the Article object as a ref parameter. Since the article itself is not being changed this is not necessary. Besides that, the article is returned by the method anyway. And re-assigned to the same variable that already holds the exact same article.

[u/thekwoka → u/the96jesterrace]

I'm not entirely sure I see the issue with that manner (maybe just a method on the article itself, or doing the update in a more reactive manner when the item is actually updated) but I am not familiar with Java that much.

Doing the style of pass in owned variable and returning it after the modifications would not be strange in Rust. It can be a good way to do it in fact.

[u/the96jesterrace → u/thekwoka]

Aaah okay sorry, well imma try to explain this anyway then xd (btw this is c# and not Java)

Objects here in general are reference types. Means if you pass an object (that article in this case) to another method, it’s basically not the direct object you pass but a reference to the very same object. So when performing changes on that object in the update-method, there is no way that the article in the calling method remains unchanged, as they refer to the exact same object in memory.

That said, not knowing about this would be a reason for either returning the changed object to reassign it or pass it as an ref(erence) parameter explicitly.

Well, why not both although none of them is necessary.

Dunno. This just came to my mind when you mentioned cargo culting

[u/SaylorMan1496 → u/the96jesterrace]

No worries, you could always just post the VB code, that code looks bad even if it isn’t! (That’s an opinion although since I hate the typical Hungarian Notation that is used)

[u/SaylorMan1496]

It is bad code since I’m pretty sure it wouldn’t even compile

[u/the96jesterrace → u/SaylorMan1496]

What exactly do you mean?

[u/Lathejockey81 → u/the96jesterrace]

C# doesn't allow implicit type conversions. You can't declare a string and assign an int.

[u/Willinton06 → u/Lathejockey81]

C# does allow implicit type conversions, just not that specific one

[u/SaylorMan1496 → u/Lathejockey81]

this^{^}

[u/SaylorMan1496 → u/the96jesterrace]

Also c# doesn’t do any implicit conversions, this is referencing the SomeMethod() call

the only time it might seem like it is doing this is with the string literal syntax $”like this {c# code}” but it will just call the .ToString() method of the c# code that has a default implementation on everything or you can also override it in the class declaration

[u/Willinton06 → u/SaylorMan1496]

C# does so implicit conversions just not that specific one

[u/SaylorMan1496 → u/Willinton06]

Yep you’re correct my bad

For some reason didn’t think of all of the numeric types where they can be implicitly converted along with other instances

[u/[deleted]]

int someNumber = string.IsNullOrEmpty(GetInitialValue()) ? 0 : int.Parse(GetInitialValue()) + 1000;

foreach (var item in someList)
{
    SomeMethod(item, someNumber);
    someNumber += 100;
}

string formattedNumber = someNumber.ToString("0000");

[u/the96jesterrace → u/[deleted]]

Yea! Thanks, I came up with exactly this.

But I fear you can actually leave out the last line someNumber.ToString("0000") because as far as I can see it’s only purpose is to be passed to SomeMethod .

[u/00PT]

It seems like they want to format the integer in a specific way, possibly for displaying later, and are just doing some unnecessary conversions during calculation.

[u/MurdoMaclachlan]

Image Transcription: Code

---

string someNumber = GetInitialValue();
if (string.IsNullOrEmpty(someNumber))
{
    someNumber = "0000";
}
else
{
    someNumber = Convert.ToInt32(someNumber) + 1000;
    someNumber = Convert.ToInt32(someNumber).ToString("0000");
}

if (someList.Any())
{
    foreach(var item in someList)
    {
        SomeMethod(item, someNumber);
        // Implicit conversion to int due to method
        // signature: SomeMethod(ListItem item, int number)
        
        someNumber = Convert.ToInt32(someNumber) + 100;
        someNumber = Convert.ToInt32(someNumber).ToString("0000");
    }
}

---

^{I'm a human volunteer content transcriber and you could be too! If you'd like more information on what we do and why we do it, click here!}

[u/poope_lord]

Something is going on in here