corrected lintr

Author: venom1204

R/merge.R

@@ -1,7 +1,7 @@
 merge.data.table = function(x, y, by = NULL, by.x = NULL, by.y = NULL, all = FALSE, all.x = all,
-                            all.y = all, sort = TRUE, suffixes = c(".x", ".y"), no.dups = TRUE,
-                            allow.cartesian = getOption("datatable.allow.cartesian"), 
-                            incomparables = NULL, ...) {
+                            all.y = all, sort = TRUE, suffixes = c(".x", ".y"), no.dups = TRUE, 
+                            allow.cartesian = getOption("datatable.allow.cartesian"), incomparables = NULL, ...) {
+  
   # Error handling for logical arguments
   if (!is.logical(sort)) stopf("Argument 'sort' should be logical TRUE/FALSE")
   if (!is.logical(no.dups)) stopf("Argument 'no.dups' should be logical TRUE/FALSE")
@@ -14,11 +14,10 @@ merge.data.table = function(x, y, by = NULL, by.x = NULL, by.y = NULL, all = FAL
       by = key(x)
     }
   }
-  
-  # Handle case when either x or y is empty
   x0 = length(x) == 0L
   y0 = length(y) == 0L
 
+  # Handle case when either x or y is empty
   if (x0 || y0) {
     if (x0 && y0) {
       warningf("Neither of the input data.tables to join have columns.")
@@ -35,49 +34,41 @@ merge.data.table = function(x, y, by = NULL, by.x = NULL, by.y = NULL, all = FAL
   nm_x = names(x)
   nm_y = names(y)
 
+  # Set up 'by'/'by.x'/'by.y'
+  if ((!is.null(by.x) || !is.null(by.y)) && length(by.x) != length(by.y)) {
+    stopf("`by.x` and `by.y` must be of same length.")
+  }
+  if (!missing(by) && !missing(by.x)) {
+    warningf("Supplied both `by` and `by.x/by.y`. `by` argument will be ignored.")
+  }
+  
   # Setup 'by', 'by.x', 'by.y'
   if ((!is.null(by.x) || !is.null(by.y)) && length(by.x) != length(by.y)) {
     stopf("by.x and by.y must be of the same length.")
   }
-  
+
   if (!missing(by) && !missing(by.x)) {
     warningf("Supplied both 'by' and 'by.x/by.y'. 'by' argument will be ignored.")
   }
-  
+
   if (!is.null(by.x)) {
     if (length(by.x) == 0L || !is.character(by.x) || !is.character(by.y)) {
-      stopf("A non-empty vector of column names is required for by.x and by.y.")
-    }
-    if (!all(by.x %chin% nm_x)) {
-      stopf("Elements listed in by.x must be valid column names in x.")
-    }
-    if (!all(by.y %chin% nm_y)) {
-      stopf("Elements listed in by.y must be valid column names in y.")
+      stopf("A non-empty vector of column names is required for `by.x` and `by.y`.")
     }
+    if (!all(by.x %chin% nm_x)) stopf("Elements listed in `by.x` must be valid column names in x.")
+    if (!all(by.y %chin% nm_y)) stopf("Elements listed in `by.y` must be valid column names in y.")
     by = by.x
     names(by) = by.y
   } else {
     if (is.null(by)) by = intersect(key(x), key(y))
     if (!length(by)) by = key(x)
     if (!length(by)) by = intersect(nm_x, nm_y)
     if (length(by) == 0L || !is.character(by)) {
-      stopf("A non-empty vector of column names for 'by' is required.")
+      stopf("A non-empty vector of column names for `by` is required.")
     }
-
-    # Updated Error Handling Section
-    missing_in_x = setdiff(by, nm_x)
-    missing_in_y = setdiff(by, nm_y)
-    if (length(missing_in_x) > 0 || length(missing_in_y) > 0) {
-      error_msg = "Columns listed in 'by' must be valid column names in both data.tables.\n"
-      if (length(missing_in_x) > 0) {
-        error_msg = paste0(error_msg, sprintf("? Missing in x: %s\n", toString(missing_in_x)))
-      }
-      if (length(missing_in_y) > 0) {
-        error_msg = paste0(error_msg, sprintf("? Missing in y: %s", toString(missing_in_y)))
-      }
-      stopf(error_msg)
+    if (!all(by %chin% intersect(nm_x, nm_y))) {
+      stopf("Elements listed in `by` must be valid column names in x and y")
     }
-
     by = unname(by)
     by.x = by.y = by
   }
@@ -87,7 +78,9 @@ merge.data.table = function(x, y, by = NULL, by.x = NULL, by.y = NULL, all = FAL
     ell = as.list(substitute(list(...)))[-1L]
     for (n in setdiff(names(ell), "")) warningf("Unknown argument '%s' has been passed.", n)
     unnamed_n = length(ell) - sum(nzchar(names(ell)))
-    if (unnamed_n) warningf("Passed %d unknown and unnamed arguments.", unnamed_n)
+    if (unnamed_n) {
+      warningf("Passed %d unknown and unnamed arguments.", unnamed_n)
+    }
   }
 
   # Handle duplicate column names
@@ -98,8 +91,13 @@ merge.data.table = function(x, y, by = NULL, by.x = NULL, by.y = NULL, all = FAL
     start[chmatch(dupnames, start, 0L)] = paste0(dupnames, suffixes[1L])
     end[chmatch(dupnames, end, 0L)] = paste0(dupnames, suffixes[2L])
   }
-  
-  # Handle incomparables argument
+  # If no.dups = TRUE we also need to add the suffix to columns in y that share a name with by.x
+  dupkeyx = intersect(by.x, end)
+  if (no.dups && length(dupkeyx)) {
+    end[chmatch(dupkeyx, end, 0L)] = paste0(dupkeyx, suffixes[2L])
+  }
+
+  # Implement incomparables argument
   if (!is.null(incomparables)) {
     "%fin%" = function(x, table) if (is.character(x) && is.character(table)) x %chin% table else x %in% table
     xind = rowSums(x[, lapply(.SD, function(x) !(x %fin% incomparables)), .SDcols = by.x]) == length(by)
@@ -109,28 +107,30 @@ merge.data.table = function(x, y, by = NULL, by.x = NULL, by.y = NULL, all = FAL
   }
 
   dt = y[x, nomatch = if (all.x) NA else NULL, on = by, allow.cartesian = allow.cartesian]
-    
+  
   if (all.y && nrow(y)) {
     missingyidx = y[!x, which = TRUE, on = by, allow.cartesian = allow.cartesian]
-    if (length(missingyidx)) dt = rbind(dt, y[missingyidx], use.names = FALSE, fill = TRUE, ignore.attr = TRUE)
+    if (length(missingyidx)) {
+      dt = rbind(dt, y[missingyidx], use.names = FALSE, fill = TRUE, ignore.attr = TRUE)
+    }
   }
 
   # Reorder columns
   newend = setdiff(nm_y, by.y)
   setcolorder(dt, c(by.y, setdiff(names(dt), c(by.y, newend)), newend))
   setnames(dt, c(by.x, start, end))
-  
+
   if (nrow(dt) > 0L) {
     setkeyv(dt, if (sort) by.x else NULL)
   }
-  
+
   # Warn about duplicate column names in result
   resultdupnames = names(dt)[duplicated(names(dt))]
   if (length(resultdupnames)) {
     warningf("Column names %s are duplicated in the result", toString(resultdupnames))
   }
 
-  # Retain custom classes
+  # Retain custom classes of first argument
   setattr(dt, "class", class_x)
   dt
 }