Naming guard clauses that throw exceptions

I have a function evaluate() that parses a String for some variables and replaces them with their corresponding value:

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public String evaluate() {
String result = templateText;
for (Entry<String, String> entry : variables.entrySet()) {
String regex = "\$\{" + entry.getKey() + "\}";
result = result.replaceAll(regex, entry.getValue());
}
if (result.matches(".*\$\{.+\}.*")) { //<--check if a variable wasn't replaced
throw new MissingValueException();
}
return result;
}
</code>
<code>public String evaluate() { String result = templateText; for (Entry<String, String> entry : variables.entrySet()) { String regex = "\$\{" + entry.getKey() + "\}"; result = result.replaceAll(regex, entry.getValue()); } if (result.matches(".*\$\{.+\}.*")) { //<--check if a variable wasn't replaced throw new MissingValueException(); } return result; } </code>
public String evaluate() {
    String result = templateText;
    for (Entry<String, String> entry : variables.entrySet()) {
        String regex = "\$\{" + entry.getKey() + "\}";
        result = result.replaceAll(regex, entry.getValue());
    }
    if (result.matches(".*\$\{.+\}.*")) {    //<--check if a variable wasn't replaced
        throw new MissingValueException();
    }
    return result;
}

Since the evaluate() function now does 2 things, it first replaces the variables with the values AND check for any variables that weren’t replaced, I thought about refactoring it to:

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public String evaluate() {
String result = templateText;
for (Entry<String, String> entry : variables.entrySet()) {
String regex = "\$\{" + entry.getKey() + "\}";
result = result.replaceAll(regex, entry.getValue());
}
checkForMissingValues(result);
}
private void checkForMissingValues(String result) {
if (result.matches(".*\$\{.+\}.*")) {
throw new MissingValueException();
}
}
</code>
<code>public String evaluate() { String result = templateText; for (Entry<String, String> entry : variables.entrySet()) { String regex = "\$\{" + entry.getKey() + "\}"; result = result.replaceAll(regex, entry.getValue()); } checkForMissingValues(result); } private void checkForMissingValues(String result) { if (result.matches(".*\$\{.+\}.*")) { throw new MissingValueException(); } } </code>
public String evaluate() {
    String result = templateText;
    for (Entry<String, String> entry : variables.entrySet()) {
        String regex = "\$\{" + entry.getKey() + "\}";
        result = result.replaceAll(regex, entry.getValue());
    }
    checkForMissingValues(result);
}

private void checkForMissingValues(String result) {
    if (result.matches(".*\$\{.+\}.*")) {
        throw new MissingValueException();
    }
}

Now is this a good name for the function checkForMissingValues? I mean it does check for missing values, but it also throws an exception.

I thought about renaming it to throwExceptionIfMissingValueWasFound(), but this name tells HOW the function does it more than WHAT the function is doing.

Is there a standard for naming such functions that check for a condition then throw an exception?

2

Why don’t you call it guardAgainstMissingValues() it’s simple and conveys the message directly.

Just ask yourself what does this function do? It guards against Missing values. You said it yourself that it’s a guard clause, so why don’t you name it accordingly.

In addition I think that the term guard clause implies a check in its implementation, so you won’t have to explicitly mention the check in the function’s name.

I usually use ensureThisOrThatAttribute, which, in combination with an explicit throws CustomMadeException declaration makes things clear enough.

However, more important than conforming to some alleged general standard is that you conform to your own code base’s standard. If you don’t have one yet, well, just pick a format now and stick with it.

1

I usually prefer a convention such as this:

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>static class Guard {
public static void IsNotNull(value) {
if(value == null) {
throw new MyCustomGuardException();
}
}
}
</code>
<code>static class Guard { public static void IsNotNull(value) { if(value == null) { throw new MyCustomGuardException(); } } } </code>
static class Guard {
  public static void IsNotNull(value) {
    if(value == null) {
      throw new MyCustomGuardException();
    }
  }
}

Later on when using it:

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public function DoSomething(DateTime startedAt) {
Guard.IsNotNull(startedAt);
}
</code>
<code>public function DoSomething(DateTime startedAt) { Guard.IsNotNull(startedAt); } </code>
public function DoSomething(DateTime startedAt) {
  Guard.IsNotNull(startedAt);
}

A more complete example of a fluent guard class can be found in the NetFX package here:

http://netfx.codeplex.com/SourceControl/changeset/view/89b72bee718c#Extensions/System/Guard/Source/content/netfx/System/Guard.cs

9

Your routine name seems perfectly fine to me. What’s so bad about “check”?

I tend to break it up, though. So I have a routine that does the check/throw, plus functions that do the actual tests. You’d wind up with something like:

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>check(!hasMissingValues(result));
</code>
<code>check(!hasMissingValues(result)); </code>
check(!hasMissingValues(result));

This makes it more than just a naming convention. You can call it “validate” or “enforce” or whatever, but the key it that check has a defined behavior.

(This of course assumes that the kinds of conditions you’re checking don’t need a bunch of custom exception info, but that is often the case.)

I don’t think this warrants a separate method.
I also noticed several things:

You create a Pattern object for each variable in your Map. Pattern object creation is expensive (requires parsing). You then evaluate the regex on the string with replaceAll. So if you have 100 variables that becomes 100 regexes parsed and created then string is scanned 100 times. That’s inefficient.

Your regex to scan for missing variables is exceedingly expensive. It will scan each character in your string many many many times, especially if it contains ${.

What you need to do is flip the logic. Find strings ${x} in text and retrieve variables from your map to fill that, throwing an exception the first time you encounter a missing variable.

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public String evaluate() {
Matcher matcher = Pattern.compile("\$\{(\.+?\)}").matcher(templateText);
StringBuilder result = new StringBuilder(templateText.length());
while(matcher.find()) {
String val = variables.get(matcher.group(1));
if (val == null) throw new MissingValueException();
matcher.appendReplacement(result, val);
}
matcher.appendTail(result);
return result.toString();
}
</code>
<code>public String evaluate() { Matcher matcher = Pattern.compile("\$\{(\.+?\)}").matcher(templateText); StringBuilder result = new StringBuilder(templateText.length()); while(matcher.find()) { String val = variables.get(matcher.group(1)); if (val == null) throw new MissingValueException(); matcher.appendReplacement(result, val); } matcher.appendTail(result); return result.toString(); } </code>
public String evaluate() {
    Matcher matcher = Pattern.compile("\$\{(\.+?\)}").matcher(templateText);
    StringBuilder result = new StringBuilder(templateText.length());
    while(matcher.find()) {
        String val = variables.get(matcher.group(1));
        if (val == null) throw new MissingValueException();
        matcher.appendReplacement(result, val);
    }
    matcher.appendTail(result);
    return result.toString();
}

This will search the string only once with only a single regex. It will fail on first missing variable. If you have 100 variables defined but only 1 appears in the template only 1 replace is done.

Well first of all, methods in C# follow the Pascal convention, so it should be Evaluate and not evaluate.

Second: It is natural for methods to throw exceptions in, well, exceptional scenarios so you DONT have to name your method to signify this.

Third: It is not normal to throw an exception due to an internal check (unless you have a specific need). Ideally the method should return a bool which states whether the check succeeded or failed. So the method signature should ideally be:

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>private bool IsMissingValues(String result)
</code>
<code>private bool IsMissingValues(String result) </code>
private bool IsMissingValues(String result) 

Next, a method name does not always have to explain every single detail of what it does, always read it in conjunction with the class in which it is defined.

Hence, String.Replace is sufficient; String.ReplaceThisStringWithAnother is overkill.

Speaking of which, your evaluate method should ideally be called Replace or something as that it what it essentially does.

Also private methods can be relatively less ‘wordy’ than public ones since no one else except you will see it (ok fine, that’s debatable).

Plain text
Copy to clipboard
Open code in new window
EnlighterJS 3 Syntax Highlighter
<code>public String evaluate() {
String result = replaceVariablesWithValues();
// Extract the for loop code to the new method listed above
if (resultContainsVariables(result)) {
// All variables should have been replaced
throw new MissingValueException();
}
return result;
}
</code>
<code>public String evaluate() { String result = replaceVariablesWithValues(); // Extract the for loop code to the new method listed above if (resultContainsVariables(result)) { // All variables should have been replaced throw new MissingValueException(); } return result; } </code>
public String evaluate() {
    String result = replaceVariablesWithValues();
       // Extract the for loop code to the new method listed above

    if (resultContainsVariables(result)) {    
         // All variables should have been replaced
        throw new MissingValueException();
    }
    return result;
}

Above is my proposal for how to refactor.

  1. The proposed evaluate function describes succinctly both the general functionality (replace variables with values) as well as the exception case (if the result contains variables, throw an exception).
  2. The details of the regex evaluations should be extracted to helper functions because one should not need to mentally parse the regex to understand the contract and control flow of evaluate. Also, I suggest making the regex prefix and suffix constants.
  3. I suggest keeping the throw statement in evaluate. It is not obvious what a “check” or guard function does, but knowing what such a function does is required to understand the contract of evaluate.

Trang chủ Giới thiệu Sinh nhật bé trai Sinh nhật bé gái Tổ chức sự kiện Biểu diễn giải trí Dịch vụ khác Trang trí tiệc cưới Tổ chức khai trương Tư vấn dịch vụ Thư viện ảnh Tin tức - sự kiện Liên hệ Chú hề sinh nhật Trang trí YEAR END PARTY công ty Trang trí tất niên cuối năm Trang trí tất niên xu hướng mới nhất Trang trí sinh nhật bé trai Hải Đăng Trang trí sinh nhật bé Khánh Vân Trang trí sinh nhật Bích Ngân Trang trí sinh nhật bé Thanh Trang Thuê ông già Noel phát quà Biểu diễn xiếc khỉ Xiếc quay đĩa Dịch vụ tổ chức sự kiện 5 sao Thông tin về chúng tôi Dịch vụ sinh nhật bé trai Dịch vụ sinh nhật bé gái Sự kiện trọn gói Các tiết mục giải trí Dịch vụ bổ trợ Tiệc cưới sang trọng Dịch vụ khai trương Tư vấn tổ chức sự kiện Hình ảnh sự kiện Cập nhật tin tức Liên hệ ngay Thuê chú hề chuyên nghiệp Tiệc tất niên cho công ty Trang trí tiệc cuối năm Tiệc tất niên độc đáo Sinh nhật bé Hải Đăng Sinh nhật đáng yêu bé Khánh Vân Sinh nhật sang trọng Bích Ngân Tiệc sinh nhật bé Thanh Trang Dịch vụ ông già Noel Xiếc thú vui nhộn Biểu diễn xiếc quay đĩa Dịch vụ tổ chức tiệc uy tín Khám phá dịch vụ của chúng tôi Tiệc sinh nhật cho bé trai Trang trí tiệc cho bé gái Gói sự kiện chuyên nghiệp Chương trình giải trí hấp dẫn Dịch vụ hỗ trợ sự kiện Trang trí tiệc cưới đẹp Khởi đầu thành công với khai trương Chuyên gia tư vấn sự kiện Xem ảnh các sự kiện đẹp Tin mới về sự kiện Kết nối với đội ngũ chuyên gia Chú hề vui nhộn cho tiệc sinh nhật Ý tưởng tiệc cuối năm Tất niên độc đáo Trang trí tiệc hiện đại Tổ chức sinh nhật cho Hải Đăng Sinh nhật độc quyền Khánh Vân Phong cách tiệc Bích Ngân Trang trí tiệc bé Thanh Trang Thuê dịch vụ ông già Noel chuyên nghiệp Xem xiếc khỉ đặc sắc Xiếc quay đĩa thú vị
Trang chủ Giới thiệu Sinh nhật bé trai Sinh nhật bé gái Tổ chức sự kiện Biểu diễn giải trí Dịch vụ khác Trang trí tiệc cưới Tổ chức khai trương Tư vấn dịch vụ Thư viện ảnh Tin tức - sự kiện Liên hệ Chú hề sinh nhật Trang trí YEAR END PARTY công ty Trang trí tất niên cuối năm Trang trí tất niên xu hướng mới nhất Trang trí sinh nhật bé trai Hải Đăng Trang trí sinh nhật bé Khánh Vân Trang trí sinh nhật Bích Ngân Trang trí sinh nhật bé Thanh Trang Thuê ông già Noel phát quà Biểu diễn xiếc khỉ Xiếc quay đĩa
Thiết kế website Thiết kế website Thiết kế website Cách kháng tài khoản quảng cáo Mua bán Fanpage Facebook Dịch vụ SEO Tổ chức sinh nhật