Skip to main content
deleted 5 characters in body
Source Link
Carcigenicate
  • 16.6k
  • 3
  • 37
  • 82

This is fairly minor, but I don't think countGlobalSizes is ideal. Its use in the constructor requires that the assignment of globalSizesranges happens first; which may cause breakage if you refactor later. It also barely has any reliance on individual instances. I'd make it static and accept the ranges directly as an argument:

private static int sumRangeLengths(Range[] ranges) {
    int totalSize = 0;

    for (Range r : ranges) {
        totalSize += r.size();
    }

    return totalSize;
}

I'm also not a fan of the word "global" being used everywhere. The variables aren't really "global" in most senses. They're private members of instances; which is a pretty constrained scope. They're global to the instance; but every member is, so that's redundant.


You may also want to add a step to your range. It's fairly trivial to implement, and is a fairly common aspect of most range implementations.

This is fairly minor, but I don't think countGlobalSizes is ideal. Its use in the constructor requires that the assignment of globalSizes happens first; which may cause breakage if you refactor later. It also barely has any reliance on individual instances. I'd make it static and accept the ranges directly as an argument:

private static int sumRangeLengths(Range[] ranges) {
    int totalSize = 0;

    for (Range r : ranges) {
        totalSize += r.size();
    }

    return totalSize;
}

I'm also not a fan of the word "global" being used everywhere. The variables aren't really "global" in most senses. They're private members of instances; which is a pretty constrained scope. They're global to the instance; but every member is, so that's redundant.


You may also want to add a step to your range. It's fairly trivial to implement, and is a fairly common aspect of most range implementations.

This is fairly minor, but I don't think countGlobalSizes is ideal. Its use in the constructor requires that the assignment of ranges happens first; which may cause breakage if you refactor later. It also barely has any reliance on individual instances. I'd make it static and accept the ranges directly as an argument:

private static int sumRangeLengths(Range[] ranges) {
    int totalSize = 0;

    for (Range r : ranges) {
        totalSize += r.size();
    }

    return totalSize;
}

I'm also not a fan of the word "global" being used everywhere. The variables aren't really "global" in most senses. They're private members of instances; which is a pretty constrained scope. They're global to the instance; but every member is, so that's redundant.


You may also want to add a step to your range. It's fairly trivial to implement, and is a fairly common aspect of most range implementations.

deleted 257 characters in body
Source Link
Carcigenicate
  • 16.6k
  • 3
  • 37
  • 82

This is fairly minor, but I don't think countGlobalSizes is ideal. Its use in the constructor requires that the assignment of globalSizes happens first; which may cause breakage if you refactor later. It also barely has any reliance on individual instances. I'd make it static and accept the ranges directly as an argument:

private static int sumRangeLengths(Range[] ranges) {
    int totalSize = 0;

    for (Range r : ranges) {
        totalSize += r.size();
    }

    return totalSize;
}

I'm also not a fan of the word "global" being used everywhere. The variables aren't really "global" in most senses. They're private members of instances; which is a pretty constrained scope. They're global to the instance; but every member is, so that's redundant.


You may also want to make copy of ranges in your constructors. You're internalizing a reference to a mutable array that was created externally. I can see things potentially breaking if the caller were to alter their array after passing it in.


You may also want to add a step to your range. It's fairly trivial to implement, and is a fairly common aspect of most range implementations.

This is fairly minor, but I don't think countGlobalSizes is ideal. Its use in the constructor requires that the assignment of globalSizes happens first; which may cause breakage if you refactor later. It also barely has any reliance on individual instances. I'd make it static and accept the ranges directly as an argument:

private static int sumRangeLengths(Range[] ranges) {
    int totalSize = 0;

    for (Range r : ranges) {
        totalSize += r.size();
    }

    return totalSize;
}

I'm also not a fan of the word "global" being used everywhere. The variables aren't really "global" in most senses. They're private members of instances; which is a pretty constrained scope. They're global to the instance; but every member is, so that's redundant.


You may also want to make copy of ranges in your constructors. You're internalizing a reference to a mutable array that was created externally. I can see things potentially breaking if the caller were to alter their array after passing it in.


You may also want to add a step to your range. It's fairly trivial to implement, and is a fairly common aspect of most range implementations.

This is fairly minor, but I don't think countGlobalSizes is ideal. Its use in the constructor requires that the assignment of globalSizes happens first; which may cause breakage if you refactor later. It also barely has any reliance on individual instances. I'd make it static and accept the ranges directly as an argument:

private static int sumRangeLengths(Range[] ranges) {
    int totalSize = 0;

    for (Range r : ranges) {
        totalSize += r.size();
    }

    return totalSize;
}

I'm also not a fan of the word "global" being used everywhere. The variables aren't really "global" in most senses. They're private members of instances; which is a pretty constrained scope. They're global to the instance; but every member is, so that's redundant.


You may also want to add a step to your range. It's fairly trivial to implement, and is a fairly common aspect of most range implementations.

Source Link
Carcigenicate
  • 16.6k
  • 3
  • 37
  • 82

This is fairly minor, but I don't think countGlobalSizes is ideal. Its use in the constructor requires that the assignment of globalSizes happens first; which may cause breakage if you refactor later. It also barely has any reliance on individual instances. I'd make it static and accept the ranges directly as an argument:

private static int sumRangeLengths(Range[] ranges) {
    int totalSize = 0;

    for (Range r : ranges) {
        totalSize += r.size();
    }

    return totalSize;
}

I'm also not a fan of the word "global" being used everywhere. The variables aren't really "global" in most senses. They're private members of instances; which is a pretty constrained scope. They're global to the instance; but every member is, so that's redundant.


You may also want to make copy of ranges in your constructors. You're internalizing a reference to a mutable array that was created externally. I can see things potentially breaking if the caller were to alter their array after passing it in.


You may also want to add a step to your range. It's fairly trivial to implement, and is a fairly common aspect of most range implementations.