Check if nullable int has value and compare value to another integer [closed]

.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;

up vote
1
down vote

favorite

Question might be more appropriate in StackOverflow, however I'd also like a review.

I'd like to make my code inline as I feel like this could be possible, I'm just not sure how.

int highestWeightOfParcel = 0;
if (collo.WeightGrammes.HasValue)

 if (collo.WeightGrammes > highestWeightOfParcel)
 
 highestWeightOfParcel = collo.WeightGrammes.Value;

Could I apply the same technique as this? :

int heigth = collo.HeightMm.HasValue ? collo.HeightMm.Value < 10 ? 1 : (collo.HeightMm.Value / 10) : 0,

Is there a 'better'/simpler way of writing this?

edited Jul 17 at 16:10

t3chb0t

31.8k54095

asked Jul 17 at 11:04

Paramone

1276

closed as off-topic by t3chb0t, Sam Onela, Toby Speight, Stephen Rauch, Dannnno Jul 17 at 19:51

This question appears to be off-topic. The users who voted to close gave this specific reason:

"Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." â€“ t3chb0t, Sam Onela, Toby Speight, Stephen Rauch, Dannnno

If this question can be reworded to fit the rules in the help center, please edit the question.

This question severely lacks context. The code also does not make any sense because when highestWeightOfParcel = 0 then it's the same as writing highestWeightOfParcel = collo.WeightGrammes ?? 0. This meas if it's not-null then it'll always be the highest value. Both ifs are completely unnecessary. I bet this is a part or a loop...
â€“Â t3chb0t
Jul 17 at 18:00

add a commentÂ |Â

up vote
1
down vote

favorite

Question might be more appropriate in StackOverflow, however I'd also like a review.

I'd like to make my code inline as I feel like this could be possible, I'm just not sure how.

int highestWeightOfParcel = 0;
if (collo.WeightGrammes.HasValue)

 if (collo.WeightGrammes > highestWeightOfParcel)
 
 highestWeightOfParcel = collo.WeightGrammes.Value;

Could I apply the same technique as this? :

int heigth = collo.HeightMm.HasValue ? collo.HeightMm.Value < 10 ? 1 : (collo.HeightMm.Value / 10) : 0,

Is there a 'better'/simpler way of writing this?

edited Jul 17 at 16:10

t3chb0t

31.8k54095

asked Jul 17 at 11:04

Paramone

1276

closed as off-topic by t3chb0t, Sam Onela, Toby Speight, Stephen Rauch, Dannnno Jul 17 at 19:51

This question appears to be off-topic. The users who voted to close gave this specific reason:

"Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." â€“ t3chb0t, Sam Onela, Toby Speight, Stephen Rauch, Dannnno

If this question can be reworded to fit the rules in the help center, please edit the question.

This question severely lacks context. The code also does not make any sense because when highestWeightOfParcel = 0 then it's the same as writing highestWeightOfParcel = collo.WeightGrammes ?? 0. This meas if it's not-null then it'll always be the highest value. Both ifs are completely unnecessary. I bet this is a part or a loop...
â€“Â t3chb0t
Jul 17 at 18:00

add a commentÂ |Â

up vote
1
down vote

favorite

Question might be more appropriate in StackOverflow, however I'd also like a review.

I'd like to make my code inline as I feel like this could be possible, I'm just not sure how.

int highestWeightOfParcel = 0;
if (collo.WeightGrammes.HasValue)

 if (collo.WeightGrammes > highestWeightOfParcel)
 
 highestWeightOfParcel = collo.WeightGrammes.Value;

Could I apply the same technique as this? :

int heigth = collo.HeightMm.HasValue ? collo.HeightMm.Value < 10 ? 1 : (collo.HeightMm.Value / 10) : 0,

Is there a 'better'/simpler way of writing this?

edited Jul 17 at 16:10

t3chb0t

31.8k54095

asked Jul 17 at 11:04

Paramone

1276

Question might be more appropriate in StackOverflow, however I'd also like a review.

I'd like to make my code inline as I feel like this could be possible, I'm just not sure how.

int highestWeightOfParcel = 0;
if (collo.WeightGrammes.HasValue)

 if (collo.WeightGrammes > highestWeightOfParcel)
 
 highestWeightOfParcel = collo.WeightGrammes.Value;

Could I apply the same technique as this? :

int heigth = collo.HeightMm.HasValue ? collo.HeightMm.Value < 10 ? 1 : (collo.HeightMm.Value / 10) : 0,

Is there a 'better'/simpler way of writing this?

edited Jul 17 at 16:10

t3chb0t

31.8k54095

asked Jul 17 at 11:04

Paramone

1276

edited Jul 17 at 16:10

t3chb0t

31.8k54095

edited Jul 17 at 16:10

t3chb0t

31.8k54095

edited Jul 17 at 16:10

t3chb0t

31.8k54095

asked Jul 17 at 11:04

Paramone

1276

asked Jul 17 at 11:04

Paramone

1276

asked Jul 17 at 11:04

Paramone

1276

closed as off-topic by t3chb0t, Sam Onela, Toby Speight, Stephen Rauch, Dannnno Jul 17 at 19:51

This question appears to be off-topic. The users who voted to close gave this specific reason:

"Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." â€“ t3chb0t, Sam Onela, Toby Speight, Stephen Rauch, Dannnno

If this question can be reworded to fit the rules in the help center, please edit the question.

closed as off-topic by t3chb0t, Sam Onela, Toby Speight, Stephen Rauch, Dannnno Jul 17 at 19:51

This question appears to be off-topic. The users who voted to close gave this specific reason:

"Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." â€“ t3chb0t, Sam Onela, Toby Speight, Stephen Rauch, Dannnno

If this question can be reworded to fit the rules in the help center, please edit the question.

This question severely lacks context. The code also does not make any sense because when highestWeightOfParcel = 0 then it's the same as writing highestWeightOfParcel = collo.WeightGrammes ?? 0. This meas if it's not-null then it'll always be the highest value. Both ifs are completely unnecessary. I bet this is a part or a loop...
â€“Â t3chb0t
Jul 17 at 18:00

add a commentÂ |Â

This question severely lacks context. The code also does not make any sense because when highestWeightOfParcel = 0 then it's the same as writing highestWeightOfParcel = collo.WeightGrammes ?? 0. This meas if it's not-null then it'll always be the highest value. Both ifs are completely unnecessary. I bet this is a part or a loop...
â€“Â t3chb0t
Jul 17 at 18:00

This question severely lacks context. The code also does not make any sense because when highestWeightOfParcel = 0 then it's the same as writing highestWeightOfParcel = collo.WeightGrammes ?? 0. This meas if it's not-null then it'll always be the highest value. Both ifs are completely unnecessary. I bet this is a part or a loop...
â€“Â t3chb0t
Jul 17 at 18:00

add a commentÂ |Â

3 Answers
3

active

oldest

votes

up vote
1
down vote

accepted

int highestWeightOfParcel = (collo.WeightGrammes.HasValue && (collo.WeightGrammes > highestWeightOfParcel))? collo.WeightGrammes.Value:0;

That line code can perhaps be written as above. However, ternary operator tends to become unreadable fairly quickly. Alternate option could be this:

int highestWeightOfParcel = 0;
if ((collo.WeightGrammes.HasValue) && 
 (collo.WeightGrammes > highestWeightOfParcel)) 
 highestWeightOfParcel = collo.WeightGrammes.Value;

answered Jul 17 at 11:25

danish

1392

Awesome, thank you! I have to agree with you, though. It does get somewhat unreadable as it goes on and on..
â€“Â Paramone
Jul 17 at 11:27

1

I disagree with your assessment of the conditional operator. Your proposed code is worse, for a number of reasons Ã¢Â€Â” but foremost because youÃ¢Â€Â™re assigning to highestWeightOfParcel twice, which I would immediately flag as code smell in a code review. Even though C# doesnÃ¢Â€Â™t generally support this, it simplifies code tremendously if you consider every variable as readonly. ThatÃ¢Â€Â™s why things like ?. and ?? were added to the language.
â€“Â Konrad Rudolph
Jul 17 at 17:29

@KonradRudolph erm..you need to declare highestWeightOfParcel ahead of if block. to use it outside the if block. Null conditional operator (?./?) are to eliminate null checks and keep code thread safe. While null coalesce operator again is to be used in case of null checks. The if statement in sample code it not limited to null check only and thus disqualify these operators. As for ternary operator being unreadable, that is purely subjective so to each one his own.
â€“Â danish
Jul 18 at 4:45

@danish You don't need to declare it beforehand, since you don't need to use it inside the conditional. In fact this comparison makes absolutely no sense here. Compare to the literal value. And use Math.Max instead.
â€“Â Konrad Rudolph
Jul 18 at 6:46

add a commentÂ |Â

up vote
16
down vote

There is really no need to check HasValue in this situation. It is sufficient to compare with highestWeightOfParcel because if collo.WeightGrammes is null the comparison is false for any value of highestWeightOfParcel:

int highestWeightOfParcel = 0;

if (collo.WeightGrammes > highestWeightOfParcel)

 highestWeightOfParcel = collo.WeightGrammes.Value;

answered Jul 17 at 13:32

Henrik Hansen

3,7781417

1

+1, was just writing the same, you were faster :)
â€“Â firda
Jul 17 at 13:32

2

+1, Thank you! I thought that, if I wouldn't check for the value, it would throw a null pointer exception whenever collo.WeightGrammes doesn't have a value, and you're trying to compare it. However, it does make sense!
â€“Â Paramone
Jul 17 at 14:49

1

@Paramone Nullable<T> will NEVER throw NullReferenceException if you access its Value property when it has not a value. If this happen then to be null is collo (assuming it's a reference type) and for that you have ?.. That said THIS is the right code, compiler does everything correctly (not a surprise). If you REALLY want a single line then your first snippet MIGHT be rewritten as highestWeightOfParcel = Math.Max(highestWeightOfParcel, collo.WeightGrammes ?? 0) (assuming highestWeightOfParcel cannot be negative).
â€“Â Adriano Repetti
Jul 17 at 15:51

Same comment as under the accepted answer: this code assigns twice to the same variable. ThatÃ¢Â€Â™s code smell and, although it might be seen as justified in this code, itÃ¢Â€Â™s really unnecessary. Treat all variables as readonly, this dramatically simplifies code flow.
â€“Â Konrad Rudolph
Jul 17 at 17:31

@konrad it's a reasonable guess to assume that =0 is fictional otherwise the whole code is a smell. Single assignment is a golden rule (especially if you do not force it chaining complex expressions and multiple ternary operators like in the other OP's example). It has however, other disadvantages (often it's about readability and quick review of test coverage). Of course to introduce a function is even better.
â€“Â Adriano Repetti
Jul 17 at 17:48

Â |Â
show 2 more comments

up vote
0
down vote

Depending on the version of C# you are using...

collo?.WeightGrammes > 0 ? collo.WeightGrammes.Value : 0

?. is null propagation. If 'collo' is null, then 'collo?.X' is also null.
null is not >0

If you are working your way through a list of values... then you can use linq

List<Collo> list= new List<Collo> ()... some values ... ;
var max = list.Max ( ( c ) => c?.WeightGrammes ) ?? 0;

answered Jul 17 at 13:21

Nigel Thorne

30328

collo.WeightGrammes is of type int?. The first if is to check whether the property is null, not collo as a whole. collo?.WeigthGrammes could be replaced with collo.WeightGrammes ?? 0 though.
â€“Â JAD
Jul 17 at 14:23

add a commentÂ |Â

3 Answers
3

active

oldest

votes

3 Answers
3

active

oldest

votes

up vote
1
down vote

accepted

int highestWeightOfParcel = (collo.WeightGrammes.HasValue && (collo.WeightGrammes > highestWeightOfParcel))? collo.WeightGrammes.Value:0;

That line code can perhaps be written as above. However, ternary operator tends to become unreadable fairly quickly. Alternate option could be this:

int highestWeightOfParcel = 0;
if ((collo.WeightGrammes.HasValue) && 
 (collo.WeightGrammes > highestWeightOfParcel)) 
 highestWeightOfParcel = collo.WeightGrammes.Value;

answered Jul 17 at 11:25

danish

1392

Awesome, thank you! I have to agree with you, though. It does get somewhat unreadable as it goes on and on..
â€“Â Paramone
Jul 17 at 11:27

1

I disagree with your assessment of the conditional operator. Your proposed code is worse, for a number of reasons Ã¢Â€Â” but foremost because youÃ¢Â€Â™re assigning to highestWeightOfParcel twice, which I would immediately flag as code smell in a code review. Even though C# doesnÃ¢Â€Â™t generally support this, it simplifies code tremendously if you consider every variable as readonly. ThatÃ¢Â€Â™s why things like ?. and ?? were added to the language.
â€“Â Konrad Rudolph
Jul 17 at 17:29

@KonradRudolph erm..you need to declare highestWeightOfParcel ahead of if block. to use it outside the if block. Null conditional operator (?./?) are to eliminate null checks and keep code thread safe. While null coalesce operator again is to be used in case of null checks. The if statement in sample code it not limited to null check only and thus disqualify these operators. As for ternary operator being unreadable, that is purely subjective so to each one his own.
â€“Â danish
Jul 18 at 4:45

@danish You don't need to declare it beforehand, since you don't need to use it inside the conditional. In fact this comparison makes absolutely no sense here. Compare to the literal value. And use Math.Max instead.
â€“Â Konrad Rudolph
Jul 18 at 6:46

add a commentÂ |Â

up vote
1
down vote

accepted

int highestWeightOfParcel = (collo.WeightGrammes.HasValue && (collo.WeightGrammes > highestWeightOfParcel))? collo.WeightGrammes.Value:0;

That line code can perhaps be written as above. However, ternary operator tends to become unreadable fairly quickly. Alternate option could be this:

int highestWeightOfParcel = 0;
if ((collo.WeightGrammes.HasValue) && 
 (collo.WeightGrammes > highestWeightOfParcel)) 
 highestWeightOfParcel = collo.WeightGrammes.Value;

answered Jul 17 at 11:25

danish

1392

Awesome, thank you! I have to agree with you, though. It does get somewhat unreadable as it goes on and on..
â€“Â Paramone
Jul 17 at 11:27

1

I disagree with your assessment of the conditional operator. Your proposed code is worse, for a number of reasons Ã¢Â€Â” but foremost because youÃ¢Â€Â™re assigning to highestWeightOfParcel twice, which I would immediately flag as code smell in a code review. Even though C# doesnÃ¢Â€Â™t generally support this, it simplifies code tremendously if you consider every variable as readonly. ThatÃ¢Â€Â™s why things like ?. and ?? were added to the language.
â€“Â Konrad Rudolph
Jul 17 at 17:29

@KonradRudolph erm..you need to declare highestWeightOfParcel ahead of if block. to use it outside the if block. Null conditional operator (?./?) are to eliminate null checks and keep code thread safe. While null coalesce operator again is to be used in case of null checks. The if statement in sample code it not limited to null check only and thus disqualify these operators. As for ternary operator being unreadable, that is purely subjective so to each one his own.
â€“Â danish
Jul 18 at 4:45

@danish You don't need to declare it beforehand, since you don't need to use it inside the conditional. In fact this comparison makes absolutely no sense here. Compare to the literal value. And use Math.Max instead.
â€“Â Konrad Rudolph
Jul 18 at 6:46

add a commentÂ |Â

up vote
1
down vote

accepted

int highestWeightOfParcel = (collo.WeightGrammes.HasValue && (collo.WeightGrammes > highestWeightOfParcel))? collo.WeightGrammes.Value:0;

That line code can perhaps be written as above. However, ternary operator tends to become unreadable fairly quickly. Alternate option could be this:

int highestWeightOfParcel = 0;
if ((collo.WeightGrammes.HasValue) && 
 (collo.WeightGrammes > highestWeightOfParcel)) 
 highestWeightOfParcel = collo.WeightGrammes.Value;

answered Jul 17 at 11:25

danish

1392

int highestWeightOfParcel = (collo.WeightGrammes.HasValue && (collo.WeightGrammes > highestWeightOfParcel))? collo.WeightGrammes.Value:0;

That line code can perhaps be written as above. However, ternary operator tends to become unreadable fairly quickly. Alternate option could be this:

int highestWeightOfParcel = 0;
if ((collo.WeightGrammes.HasValue) && 
 (collo.WeightGrammes > highestWeightOfParcel)) 
 highestWeightOfParcel = collo.WeightGrammes.Value;

answered Jul 17 at 11:25

danish

1392

answered Jul 17 at 11:25

danish

1392

answered Jul 17 at 11:25

danish

1392

answered Jul 17 at 11:25

danish

1392

Awesome, thank you! I have to agree with you, though. It does get somewhat unreadable as it goes on and on..
â€“Â Paramone
Jul 17 at 11:27

1

I disagree with your assessment of the conditional operator. Your proposed code is worse, for a number of reasons Ã¢Â€Â” but foremost because youÃ¢Â€Â™re assigning to highestWeightOfParcel twice, which I would immediately flag as code smell in a code review. Even though C# doesnÃ¢Â€Â™t generally support this, it simplifies code tremendously if you consider every variable as readonly. ThatÃ¢Â€Â™s why things like ?. and ?? were added to the language.
â€“Â Konrad Rudolph
Jul 17 at 17:29

@KonradRudolph erm..you need to declare highestWeightOfParcel ahead of if block. to use it outside the if block. Null conditional operator (?./?) are to eliminate null checks and keep code thread safe. While null coalesce operator again is to be used in case of null checks. The if statement in sample code it not limited to null check only and thus disqualify these operators. As for ternary operator being unreadable, that is purely subjective so to each one his own.
â€“Â danish
Jul 18 at 4:45

@danish You don't need to declare it beforehand, since you don't need to use it inside the conditional. In fact this comparison makes absolutely no sense here. Compare to the literal value. And use Math.Max instead.
â€“Â Konrad Rudolph
Jul 18 at 6:46

add a commentÂ |Â

Awesome, thank you! I have to agree with you, though. It does get somewhat unreadable as it goes on and on..
â€“Â Paramone
Jul 17 at 11:27

1

I disagree with your assessment of the conditional operator. Your proposed code is worse, for a number of reasons Ã¢Â€Â” but foremost because youÃ¢Â€Â™re assigning to highestWeightOfParcel twice, which I would immediately flag as code smell in a code review. Even though C# doesnÃ¢Â€Â™t generally support this, it simplifies code tremendously if you consider every variable as readonly. ThatÃ¢Â€Â™s why things like ?. and ?? were added to the language.
â€“Â Konrad Rudolph
Jul 17 at 17:29

@KonradRudolph erm..you need to declare highestWeightOfParcel ahead of if block. to use it outside the if block. Null conditional operator (?./?) are to eliminate null checks and keep code thread safe. While null coalesce operator again is to be used in case of null checks. The if statement in sample code it not limited to null check only and thus disqualify these operators. As for ternary operator being unreadable, that is purely subjective so to each one his own.
â€“Â danish
Jul 18 at 4:45

@danish You don't need to declare it beforehand, since you don't need to use it inside the conditional. In fact this comparison makes absolutely no sense here. Compare to the literal value. And use Math.Max instead.
â€“Â Konrad Rudolph
Jul 18 at 6:46

Awesome, thank you! I have to agree with you, though. It does get somewhat unreadable as it goes on and on..
â€“Â Paramone
Jul 17 at 11:27

I disagree with your assessment of the conditional operator. Your proposed code is worse, for a number of reasons Ã¢Â€Â” but foremost because youÃ¢Â€Â™re assigning to highestWeightOfParcel twice, which I would immediately flag as code smell in a code review. Even though C# doesnÃ¢Â€Â™t generally support this, it simplifies code tremendously if you consider every variable as readonly. ThatÃ¢Â€Â™s why things like ?. and ?? were added to the language.
â€“Â Konrad Rudolph
Jul 17 at 17:29

@KonradRudolph erm..you need to declare highestWeightOfParcel ahead of if block. to use it outside the if block. Null conditional operator (?./?) are to eliminate null checks and keep code thread safe. While null coalesce operator again is to be used in case of null checks. The if statement in sample code it not limited to null check only and thus disqualify these operators. As for ternary operator being unreadable, that is purely subjective so to each one his own.
â€“Â danish
Jul 18 at 4:45

@danish You don't need to declare it beforehand, since you don't need to use it inside the conditional. In fact this comparison makes absolutely no sense here. Compare to the literal value. And use Math.Max instead.
â€“Â Konrad Rudolph
Jul 18 at 6:46

add a commentÂ |Â

up vote
16
down vote

int highestWeightOfParcel = 0;

if (collo.WeightGrammes > highestWeightOfParcel)

 highestWeightOfParcel = collo.WeightGrammes.Value;

answered Jul 17 at 13:32

Henrik Hansen

3,7781417

1

+1, was just writing the same, you were faster :)
â€“Â firda
Jul 17 at 13:32

2

+1, Thank you! I thought that, if I wouldn't check for the value, it would throw a null pointer exception whenever collo.WeightGrammes doesn't have a value, and you're trying to compare it. However, it does make sense!
â€“Â Paramone
Jul 17 at 14:49

1

@Paramone Nullable<T> will NEVER throw NullReferenceException if you access its Value property when it has not a value. If this happen then to be null is collo (assuming it's a reference type) and for that you have ?.. That said THIS is the right code, compiler does everything correctly (not a surprise). If you REALLY want a single line then your first snippet MIGHT be rewritten as highestWeightOfParcel = Math.Max(highestWeightOfParcel, collo.WeightGrammes ?? 0) (assuming highestWeightOfParcel cannot be negative).
â€“Â Adriano Repetti
Jul 17 at 15:51

Same comment as under the accepted answer: this code assigns twice to the same variable. ThatÃ¢Â€Â™s code smell and, although it might be seen as justified in this code, itÃ¢Â€Â™s really unnecessary. Treat all variables as readonly, this dramatically simplifies code flow.
â€“Â Konrad Rudolph
Jul 17 at 17:31

@konrad it's a reasonable guess to assume that =0 is fictional otherwise the whole code is a smell. Single assignment is a golden rule (especially if you do not force it chaining complex expressions and multiple ternary operators like in the other OP's example). It has however, other disadvantages (often it's about readability and quick review of test coverage). Of course to introduce a function is even better.
â€“Â Adriano Repetti
Jul 17 at 17:48

Â |Â
show 2 more comments

up vote
16
down vote

int highestWeightOfParcel = 0;

if (collo.WeightGrammes > highestWeightOfParcel)

 highestWeightOfParcel = collo.WeightGrammes.Value;

answered Jul 17 at 13:32

Henrik Hansen

3,7781417

1

+1, was just writing the same, you were faster :)
â€“Â firda
Jul 17 at 13:32

2

+1, Thank you! I thought that, if I wouldn't check for the value, it would throw a null pointer exception whenever collo.WeightGrammes doesn't have a value, and you're trying to compare it. However, it does make sense!
â€“Â Paramone
Jul 17 at 14:49

1

@Paramone Nullable<T> will NEVER throw NullReferenceException if you access its Value property when it has not a value. If this happen then to be null is collo (assuming it's a reference type) and for that you have ?.. That said THIS is the right code, compiler does everything correctly (not a surprise). If you REALLY want a single line then your first snippet MIGHT be rewritten as highestWeightOfParcel = Math.Max(highestWeightOfParcel, collo.WeightGrammes ?? 0) (assuming highestWeightOfParcel cannot be negative).
â€“Â Adriano Repetti
Jul 17 at 15:51

Same comment as under the accepted answer: this code assigns twice to the same variable. ThatÃ¢Â€Â™s code smell and, although it might be seen as justified in this code, itÃ¢Â€Â™s really unnecessary. Treat all variables as readonly, this dramatically simplifies code flow.
â€“Â Konrad Rudolph
Jul 17 at 17:31

@konrad it's a reasonable guess to assume that =0 is fictional otherwise the whole code is a smell. Single assignment is a golden rule (especially if you do not force it chaining complex expressions and multiple ternary operators like in the other OP's example). It has however, other disadvantages (often it's about readability and quick review of test coverage). Of course to introduce a function is even better.
â€“Â Adriano Repetti
Jul 17 at 17:48

Â |Â
show 2 more comments

up vote
16
down vote

int highestWeightOfParcel = 0;

if (collo.WeightGrammes > highestWeightOfParcel)

 highestWeightOfParcel = collo.WeightGrammes.Value;

answered Jul 17 at 13:32

Henrik Hansen

3,7781417

int highestWeightOfParcel = 0;

if (collo.WeightGrammes > highestWeightOfParcel)

 highestWeightOfParcel = collo.WeightGrammes.Value;

answered Jul 17 at 13:32

Henrik Hansen

3,7781417

answered Jul 17 at 13:32

Henrik Hansen

3,7781417

answered Jul 17 at 13:32

Henrik Hansen

3,7781417

answered Jul 17 at 13:32

Henrik Hansen

3,7781417

1

+1, was just writing the same, you were faster :)
â€“Â firda
Jul 17 at 13:32

2

+1, Thank you! I thought that, if I wouldn't check for the value, it would throw a null pointer exception whenever collo.WeightGrammes doesn't have a value, and you're trying to compare it. However, it does make sense!
â€“Â Paramone
Jul 17 at 14:49

1

@Paramone Nullable<T> will NEVER throw NullReferenceException if you access its Value property when it has not a value. If this happen then to be null is collo (assuming it's a reference type) and for that you have ?.. That said THIS is the right code, compiler does everything correctly (not a surprise). If you REALLY want a single line then your first snippet MIGHT be rewritten as highestWeightOfParcel = Math.Max(highestWeightOfParcel, collo.WeightGrammes ?? 0) (assuming highestWeightOfParcel cannot be negative).
â€“Â Adriano Repetti
Jul 17 at 15:51

Same comment as under the accepted answer: this code assigns twice to the same variable. ThatÃ¢Â€Â™s code smell and, although it might be seen as justified in this code, itÃ¢Â€Â™s really unnecessary. Treat all variables as readonly, this dramatically simplifies code flow.
â€“Â Konrad Rudolph
Jul 17 at 17:31

@konrad it's a reasonable guess to assume that =0 is fictional otherwise the whole code is a smell. Single assignment is a golden rule (especially if you do not force it chaining complex expressions and multiple ternary operators like in the other OP's example). It has however, other disadvantages (often it's about readability and quick review of test coverage). Of course to introduce a function is even better.
â€“Â Adriano Repetti
Jul 17 at 17:48

Â |Â
show 2 more comments

1

+1, was just writing the same, you were faster :)
â€“Â firda
Jul 17 at 13:32

2

+1, Thank you! I thought that, if I wouldn't check for the value, it would throw a null pointer exception whenever collo.WeightGrammes doesn't have a value, and you're trying to compare it. However, it does make sense!
â€“Â Paramone
Jul 17 at 14:49

1

@Paramone Nullable<T> will NEVER throw NullReferenceException if you access its Value property when it has not a value. If this happen then to be null is collo (assuming it's a reference type) and for that you have ?.. That said THIS is the right code, compiler does everything correctly (not a surprise). If you REALLY want a single line then your first snippet MIGHT be rewritten as highestWeightOfParcel = Math.Max(highestWeightOfParcel, collo.WeightGrammes ?? 0) (assuming highestWeightOfParcel cannot be negative).
â€“Â Adriano Repetti
Jul 17 at 15:51

Same comment as under the accepted answer: this code assigns twice to the same variable. ThatÃ¢Â€Â™s code smell and, although it might be seen as justified in this code, itÃ¢Â€Â™s really unnecessary. Treat all variables as readonly, this dramatically simplifies code flow.
â€“Â Konrad Rudolph
Jul 17 at 17:31

@konrad it's a reasonable guess to assume that =0 is fictional otherwise the whole code is a smell. Single assignment is a golden rule (especially if you do not force it chaining complex expressions and multiple ternary operators like in the other OP's example). It has however, other disadvantages (often it's about readability and quick review of test coverage). Of course to introduce a function is even better.
â€“Â Adriano Repetti
Jul 17 at 17:48

+1, was just writing the same, you were faster :)
â€“Â firda
Jul 17 at 13:32

+1, Thank you! I thought that, if I wouldn't check for the value, it would throw a null pointer exception whenever collo.WeightGrammes doesn't have a value, and you're trying to compare it. However, it does make sense!
â€“Â Paramone
Jul 17 at 14:49

@Paramone Nullable<T> will NEVER throw NullReferenceException if you access its Value property when it has not a value. If this happen then to be null is collo (assuming it's a reference type) and for that you have ?.. That said THIS is the right code, compiler does everything correctly (not a surprise). If you REALLY want a single line then your first snippet MIGHT be rewritten as highestWeightOfParcel = Math.Max(highestWeightOfParcel, collo.WeightGrammes ?? 0) (assuming highestWeightOfParcel cannot be negative).
â€“Â Adriano Repetti
Jul 17 at 15:51

Same comment as under the accepted answer: this code assigns twice to the same variable. ThatÃ¢Â€Â™s code smell and, although it might be seen as justified in this code, itÃ¢Â€Â™s really unnecessary. Treat all variables as readonly, this dramatically simplifies code flow.
â€“Â Konrad Rudolph
Jul 17 at 17:31

@konrad it's a reasonable guess to assume that =0 is fictional otherwise the whole code is a smell. Single assignment is a golden rule (especially if you do not force it chaining complex expressions and multiple ternary operators like in the other OP's example). It has however, other disadvantages (often it's about readability and quick review of test coverage). Of course to introduce a function is even better.
â€“Â Adriano Repetti
Jul 17 at 17:48

Â |Â
show 2 more comments

up vote
0
down vote

Depending on the version of C# you are using...

collo?.WeightGrammes > 0 ? collo.WeightGrammes.Value : 0

?. is null propagation. If 'collo' is null, then 'collo?.X' is also null.
null is not >0

If you are working your way through a list of values... then you can use linq

List<Collo> list= new List<Collo> ()... some values ... ;
var max = list.Max ( ( c ) => c?.WeightGrammes ) ?? 0;

answered Jul 17 at 13:21

Nigel Thorne

30328

collo.WeightGrammes is of type int?. The first if is to check whether the property is null, not collo as a whole. collo?.WeigthGrammes could be replaced with collo.WeightGrammes ?? 0 though.
â€“Â JAD
Jul 17 at 14:23

add a commentÂ |Â

up vote
0
down vote

Depending on the version of C# you are using...

collo?.WeightGrammes > 0 ? collo.WeightGrammes.Value : 0

?. is null propagation. If 'collo' is null, then 'collo?.X' is also null.
null is not >0

If you are working your way through a list of values... then you can use linq

List<Collo> list= new List<Collo> ()... some values ... ;
var max = list.Max ( ( c ) => c?.WeightGrammes ) ?? 0;

answered Jul 17 at 13:21

Nigel Thorne

30328

collo.WeightGrammes is of type int?. The first if is to check whether the property is null, not collo as a whole. collo?.WeigthGrammes could be replaced with collo.WeightGrammes ?? 0 though.
â€“Â JAD
Jul 17 at 14:23

add a commentÂ |Â

up vote
0
down vote

Depending on the version of C# you are using...

collo?.WeightGrammes > 0 ? collo.WeightGrammes.Value : 0

?. is null propagation. If 'collo' is null, then 'collo?.X' is also null.
null is not >0

If you are working your way through a list of values... then you can use linq

List<Collo> list= new List<Collo> ()... some values ... ;
var max = list.Max ( ( c ) => c?.WeightGrammes ) ?? 0;

answered Jul 17 at 13:21

Nigel Thorne

30328

Depending on the version of C# you are using...

collo?.WeightGrammes > 0 ? collo.WeightGrammes.Value : 0

?. is null propagation. If 'collo' is null, then 'collo?.X' is also null.
null is not >0

If you are working your way through a list of values... then you can use linq

List<Collo> list= new List<Collo> ()... some values ... ;
var max = list.Max ( ( c ) => c?.WeightGrammes ) ?? 0;

answered Jul 17 at 13:21

Nigel Thorne

30328

answered Jul 17 at 13:21

Nigel Thorne

30328

answered Jul 17 at 13:21

Nigel Thorne

30328

answered Jul 17 at 13:21

Nigel Thorne

30328

collo.WeightGrammes is of type int?. The first if is to check whether the property is null, not collo as a whole. collo?.WeigthGrammes could be replaced with collo.WeightGrammes ?? 0 though.
â€“Â JAD
Jul 17 at 14:23

add a commentÂ |Â

collo.WeightGrammes is of type int?. The first if is to check whether the property is null, not collo as a whole. collo?.WeigthGrammes could be replaced with collo.WeightGrammes ?? 0 though.
â€“Â JAD
Jul 17 at 14:23

collo.WeightGrammes is of type int?. The first if is to check whether the property is null, not collo as a whole. collo?.WeigthGrammes could be replaced with collo.WeightGrammes ?? 0 though.
â€“Â JAD
Jul 17 at 14:23

add a commentÂ |Â

搜尋此網誌

trjhtr