Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Sep 22 11:57

    dennisdoomen on master

    Updated w.r.t. PR #193 (Revised… (compare)

  • Sep 22 11:57
    dennisdoomen closed #194
  • Sep 22 09:15
    keremispirli opened #194
  • Sep 22 08:58
    keremispirli commented #193
  • Sep 22 06:56

    dennisdoomen on master

    Revised AV1553 (#193) The curr… (compare)

  • Sep 22 06:56
    dennisdoomen closed #193
  • Sep 22 06:56
    dennisdoomen commented #193
  • Sep 21 22:39
    keremispirli opened #193
  • Sep 10 13:55
    bkoelman commented #178
  • Sep 10 07:29
    SierraNL commented #178
  • Sep 04 20:05

    dennisdoomen on master

    Changed guidance on #region usa… (compare)

  • Sep 04 20:05
    dennisdoomen closed #192
  • Sep 04 20:05
    dennisdoomen closed #191
  • Sep 04 19:41
    bkoelman opened #192
  • Sep 04 19:24
    dennisdoomen commented #191
  • Sep 04 17:36
    cocowalla commented #191
  • Sep 04 15:56

    dennisdoomen on master

    Small corrections Fixed script reference (compare)

  • Sep 04 15:56
    dennisdoomen closed #190
  • Sep 04 15:23
    bkoelman synchronize #190
  • Sep 04 15:22
    bkoelman opened #191
Dennis Doomen
@dennisdoomen
If you're looking at the code from inside Visual Studio...yes.... But what if you're reviewing a Pull Request on GitHub or BitBucket?
Ivan Korneliuk
@vansha
I understand that we're referring code reviews. And understand your point. Just saying that it seems like a rare case, when I can't understand type from a variable name and usage (for common examples that I can imagine):
var employees = repository.FindEmployees(criteria); // Don't really care about 'customers' type. Whether it IEnumerable<Person>, IList or List. Just know that it has a collection semantic.
var topSalary = employees.Select(var emp => emp.CalculateYearSalary()).Max(); // While I don't know exact type of 'topSalary' would be, it's not that important to me. I'm pretty sure it's primitive type like Integer  or Double.
Dennis Doomen
@dennisdoomen
Depends on the context. In your first example, I might wanna know whether somebody is returning an internal domain entity rather than some kind of DTO construct. If it is the former, you end up with unnecessary class coupling and leaky abstractions. In the second example, the difference between an integer and a double isn't really relevant, but might be considered an serious issue in accounting.
Yves Goergen
@ygoe
Ehm, wait a minute, wouldn't you detect such inappropriate return type from the method definition? When it should return an int value it surely can't return a complex internal type. You only need to check the method definition once whereas you'd have to analyse and verify its usage many times. And should the return type be something as unspecific as object you'd have another reason to complain, right?
Dennis Doomen
@dennisdoomen
The point is that I'd like to understand the type without browsing to the method definition. If you're reviewing code through e.g. Github Pull Requests, you won't see all that.
Yves Goergen
@ygoe
But when you only review the use of a method, you can't change the method itself. If you want to be able to influence the method, you need to review its definition. Once that's approved, it's safe.
You can't pull out an internal type of a method that just doesn't return it, so you can't hide it in 'var' as well.
Oleg Kleyman
@OlegKleyman

Hey Guys, with respect to AV1515. Would the literal used here be considered a magic number when the constructor is define as:

/// <param name="characterCount">The amount of characters that a segment should have.</param>
public Segment(int characterCount) and the usage is as such public class

ModelSegment : Segment { public Model() : base(4) {} }

Would the 4 above be considered magic or is being aware of the definition of the parameter be enough?

Dennis Doomen
@dennisdoomen
@OlegKleyman I would still treat that as a magic number. If you're reading this line in an online diff such as on github, you have no clue what it means. And in most cases, the base-class definition would be in a seperate file.
Oleg Kleyman
@OlegKleyman
@dennisdoomen Then how would you know if the naming is an accurate representation of the value if you're unaware what it's used for?
Dennis Doomen
@dennisdoomen
That should have been verified when the base-class is being reviewed.
Oleg Kleyman
@OlegKleyman
@dennisdoomen but if you're reviewing the child class and you don't know what the base class documentation is then wouldn't you have to look at the documentation for it anyway to verify that the constant is named correctly?
Dennis Doomen
@dennisdoomen
Well, yes, but if you name the constant something like SegmentCharacterCount, it should be pretty clear.
Oleg Kleyman
@OlegKleyman
@dennisdoomen but lets say the name of the constant is SegmentCharacterCount, but you are unaware of the base constructor it is used in. Wouldn't it mean that it's not possible review the code for correctness because you wouldn't know whether the name SegmentCharacterCount is correct? On the flip side if you are aware of what the base constructor is doing then wouldn't you be aware of what the literal represents without it being in a constant?
Dennis Doomen
@dennisdoomen
@OlegKleyman Well, at least I would give me a cue that this is about the character count. Although it wouldn't surface bugs because somebody was passing a character count to an integer parameter that has nothing to do with it ;-)
Oleg Kleyman
@OlegKleyman
er I don't understand what you mean by "it wouldn't surface bugs". Wouldn't an incorrect integer cause a bug?
Dennis Doomen
@dennisdoomen
Yeah, but if you're looking at the constructor invocation, and notice that SegmentCharacterCount, you wouldn't be able to see whether the underlying parameter represents a character count. It could be some integer with a completely different purpose.
Oleg Kleyman
@OlegKleyman
well that's my argument, if you don't know what the documentation of the base is then the name of the constant is irrelevant because you shouldn't review/modify code you don't understand
Dennis Doomen
@dennisdoomen
IMHO, the change that somebody tries to pass a character count to a variable that isn't representing that is a lot less likely. Using a named constant will at least clarify the intention.
Oleg Kleyman
@OlegKleyman
ok, thanks! I really appreciate the discussion
Maher Jendoubi
@MaherJendoubi
Hello, what about C#6 now?
Dennis Doomen
@dennisdoomen
@MaherJendoubi any suggestions are welcome
Sathanu
@sathanu
I have one question about async in c#
when i run around 20000 task at a time using task.whenall, it won't run more than 1500 task
Can anyone explain what is the issue?
Vijay
@VQuery
Hi all ,
C:\Program Files (x86)\MSBuild\14.0\bin\amd64\Microsoft.Common.CurrentVersion.targets(277,5): error MSB4184: The expression "[System.IO.Path]::GetFullPath(C:/Jenkins/workspace/DataScience/datascience-buildrequest\C:/Jenkins/workspace/DataScience/datascience-buildrequest\Assemblies\)" cannot be evaluated. The given path's format is not supported.
facing below issue in project,please help me on this
Dennis Doomen
@dennisdoomen
Which project? The Github one?
Vijay
@VQuery
@dennisdoomen , Sorry i fixed this issue by / instead of \ :smile:
Vijay
@VQuery
How to publish web projects by msbuild engine exe?
Vijay
@VQuery
:worried:
But in vs15 machine its building fine
Christian Johansen
@cjjohansen_twitter
Hi looks interesting, just joined
Dennis Doomen
@dennisdoomen
:+1:
Kris Kater
@youfoundkris
I would like to discuss AV1200.
How does AV1200 relate to the old mantra to not use exceptions for control flow?
Dennis Doomen
@dennisdoomen
I don't agree with that mantra
What I mean is that I don't like to use return values for methods that are not supposed to fail.
If they can fail under normal circumstances, I would make that explicit in the name of the method.
Or by providing a property to see if calling that method is save to be called
E.g. Save and CanSave
Kris Kater
@youfoundkris
Thanks for the reply. We are having a recurring discussion surrounding this topic at my current job.
As an example, consider the following service method:
        public async Task<IEnumerable<User>> GetUsersAsync(
            Guid customerId,
            CancellationToken token = default)
        {
            EnsureArg.IsNotEmpty(customerId, nameof(customerId));

            if (!await CompanyExistsAsync(customerId))
            {
                _logger.LogError(ExceptionMessages.UnknownCustomerException, customerId);

                throw new UnknownCustomerException(ExceptionMessages.UnknownCustomerException, customerId);
            }

            IEnumerable<Data.User> dbUsers = _userRepository.GetUsersForCustomer(customerId);

            var users = _mapper.Map<IEnumerable<User>>(dbUsers);

            return users;
        }
Does AV1200 imply that this is preferred instead of returning result objects with status codes and possibly/optionally an object? E.g. Something like Result<TEntity, TResultCode>.
Dennis Doomen
@dennisdoomen
I would throw an exception if the customer ID is invalid
If there are no users, the enumerable is just empty
Kris Kater
@youfoundkris
Thanks for the reply!
No lets assume we are on-boarding a new customer and instead of doing a check if the customer exists we verify if the chosen sub-domain name is still available. Would you use an exception or a result code?
Dennis Doomen
@dennisdoomen
Well. I would do both. First a mechanism in the UI to try to avoid an exception. But if a concurrency issue does happen, I would throw a well-defined exception.
twarnick
@twarnick_twitter
anybody here? I see only very old messages :( ?