Convert hex to base64 (Cryptopals challenge 1) I've been spending some time learning Rust and I came across the Cryptopals Challenges.
This is my implementation of challenge 1

  Convert hex to base64 The string:

49276d206b696c6c696e6720796f757220627261696e206c696b65206120706f69736f6e6f7573206d757368726f6f6d

  Should produce:

SSdtIGtpbGxpbmcgeW91ciBicmFpbiBsaWtlIGEgcG9pc29ub3VzIG11c2hyb29t

  So go ahead and make that happen. You'll need to use this code for the
  rest of the exercises.

  Cryptopals Rule: Always operate on raw bytes, never on encoded strings. Only use hex and base64 for pretty-printing.

I did go ahead and pull in a crate for handling base64 encoding. It didn't seem important to implement that myself.

mod basics {
    extern crate base64;
    use self::base64::encode;

    pub fn hex_to_base64(hex: &str) -> String {
        encode(&hex_to_bytes(hex))
    }

    pub fn hex_to_bytes(hex: &str) -> Vec<u8> {
        hex.chars()
            .collect::<Vec<_>>() //convert to slice-able
            .chunks(2)           //each char is one nibble
            .map(|byte| byte.iter().collect::<String>())
            .map(|byte| u8::from_str_radix(&byte[..], 16).unwrap())
            .collect()
    }

    #[cfg(test)]
    mod tests {
        use super::*;

        #[test]
        fn can_convert_hex_to_base64() {
            let hex_as_string =
                "49276d206b696c6c696e6720796f757220627261696e206c696b65206120706f69736f6e6f7573206d757368726f6f6d";
            let base64_as_string =
                "SSdtIGtpbGxpbmcgeW91ciBicmFpbiBsaWtlIGEgcG9pc29ub3VzIG11c2hyb29t";

            assert_eq!(hex_to_base64(hex_as_string), base64_as_string);
        }

        #[test]
        fn ff_byte_hex_string_to_byte_vector() {
            let hex = "FF";
            assert_eq!(vec![0xFF], hex_to_bytes(hex));
        }

        #[test]
        fn single_byte_hex_string_to_byte_vector() {
            let hex = "2A";
            assert_eq!(vec![0x2A], hex_to_bytes(hex));
        }

        #[test]
        fn multibyte_hex_string_to_byte_vector() {
            let hex = "2ABE";
            assert_eq!(vec![0x2A, 0xBE], hex_to_bytes(hex));
        }
    }
}

pub fn hex_to_base64(hex: &str) -> String {
    encode(&hex_to_bytes(hex))

I would not use self::base64::encode and instead refer to base64::encode here so that it is clearer what is being encoded.

}

pub fn hex_to_bytes(hex: &str) -> Vec<u8> {
    hex.chars()
        .collect::<Vec<_>>() //convert to slice-able

Given that you are doing hex, which is all in the ASCII range, you may want to use as_bytes() which will give you a slice of bytes without copying the whole string into a new Vec.

        .chunks(2)           //each char is one nibble

What happens if the string length isn't a multiple of two?

        .map(|byte| byte.iter().collect::<String>())

byte isn't correct here as it is a slice of chars. Further, if you don't use chars() as I suggest as above, you can use std::str::from_utf8 to get a &str without allocating a new String.

        .map(|byte| u8::from_str_radix(&byte[..], 16).unwrap())

byte really isn't correct here as you have a String.  Additionally, there is no need to take a slice.

        .collect()
}


Using Scanner input to do some basic math and format an output Write a static method named plusScores that accepts as a parameter a
  Scanner containing a series of lines that represent student records.
  Each student record takes up two lines of input. The first line has
  the student's name and the second line has a series of plus and minus
  characters. Below is a sample input:

Kane, Erica
--+-+
Chandler, Adam
++-+
Martin, Jake
+++++++
Dillon, Amanda
++-++-+-

  The number of plus/minus characters will vary, but you may assume that
  at least one such character appears and that no other
  characters appear on the second line of each pair. For each student
  you should produce a line of output withthe student's name followed by
  a colon followed by the percent of plus characters. For example, if
  the input above is stored in a Scanner called input, the call of
  plusScores(input); should produce the following output:

Kane, Erica: 40.0% plus
Chandler, Adam: 75.0% plus
Martin, Jake: 100.0% plus
Dillon, Amanda: 62.5% plus

I'm looking for alternative ways of solving this problem, and of course any potential bugs or pitfalls my code may have. If you have something interesting to share, please do! I'm new to programming and am eager to see other ways of accomplishing a problem such as this one. Here's my code:

public static void plusScores(Scanner sc) {
    while (sc.hasNextLine()) {
        String line = sc.nextLine();
        if (!line.startsWith("+") && !line.startsWith("-")) {
            System.out.print(line);
        } else {

            int plus = 0;
            int minus = 0;

            for (char ch : line.toCharArray()) {
                if (ch == '+') {
                    plus++;
                } else {
                    minus++;
                }
            }

            float percentPlus = ((float) (plus) / (plus + minus)) * 100;
            System.out.format(": %.1f%% plus\n", percentPlus);
        }
    }
}

Your approach is mostly correct, but it seems to be too specifically written to solve the problem as stated.

For instance, you're making the assumption that any line that doesn't start with a plus or a minus is a line containing student names.

One example case: what if a student gets no pluses or minuses? This wouldn't occur in the case as stated, but you usually don't get this "it's already handled" and it just helps to program in such a way that input which slightly deviates from normal can still be dealt with gracefully.

You're also making the assumption that when you're counting pluses and minuses, everything that is not a plus is a minus.

This works, for now. But if changes in the scoring mechanism are requested, you might want to go with a more flexible approach - one which separates the I/O and the counting, for instance.


Remove Duplicates From Linked List In Place - Java This is my implementation

class Ch02LinkedLists {
    void removeDuplicatesInPlace(Node head) {
        Node anchor = head;
        Node curr;

        while ((curr = anchor) != null) {
            while (curr.next != null)
                if (curr.next.val == anchor.val)
                    curr.next = curr.next.next;
                else
                    curr = curr.next;
            anchor = anchor.next;
        }
    }
}

class Node {
    char val;
    Node next;

    Node(char val) {
        this.val = val;
    }
}

And my test code

public class Ch02LinkedListsTest {

    Ch02LinkedLists linkedLists = new Ch02LinkedLists();

    @Test(timeout = 200) // Timeout to detect infinite loop
    public void removeDuplicatesInPlace() {
        Node a = new Node('a');
        Node a_dup = new Node('a');
        Node b = new Node('b');
        Node c = new Node('c');
        Node b_dup = new Node('b');
        Node b_dup_dup = new Node('b');
        Node c_dup = new Node('c');
        Node c_dup_dup = new Node('c');

        a.next = a_dup;
        a_dup.next = b;
        b.next = c;
        c.next = b_dup;
        b_dup.next = b_dup_dup;
        b_dup_dup.next = c_dup;
        c_dup.next = c_dup_dup;

        // a -> a -> b -> c -> b -> b -> c -> c
        linkedLists.removeDuplicatesInPlace(a);
        // a -> b -> c

        assertThat(a.val, equalTo('a'));
        assertThat(a.next.val, equalTo('b'));
        assertThat(a.next.next.val, equalTo('c'));
        assertThat(a.next.next.next, equalTo(null));
    }

}

As you see I have a single statement in my inner while hence no need for the curly braces..

while (curr.next != null)
    if (curr.next.val == anchor.val)
        curr.next = curr.next.next;
    else
        curr = curr.next;

Is there any way to achieve it for the outer loop, i.e. for:

while ((curr = anchor) != null) {
    // inner while here
    anchor = anchor.next;
}

so that I can remove the curly braces from the outer while as well?

Also, any other review is welcome.

Classes not designed for extension should be made final. Variables which should not change after initial assignment should also be made final. In addition to documenting design intent, they make it easier to read the code because you know they won't change.

As @IEatBagels said, removing curly braces is not a reasonable design goal. Code readability should be maximized. In particular, even optional curly braces are desirable in almost all cases, both because they enhance readability and because they avoid an obnoxious bug when you need to add a second statement and forget to add the curly braces.

In idiomatic java, underscores are only used in the names of constants. Prefer the use of camelCase.

For readability, it's preferable to use full, descriptive variable names rather than abbreviations.

In non-trivial code, you'd often want to encapsulate the variables in Node. This lets you change the types of those variables without breaking the contract of Node.

If you build your test list slightly differently, you don't need all those new variables. Even better would be to use a method to create the list for you.

A toString implementation on Node might be nice. Then you can just print the node directly, rather than asking for the value.

An arguably superior approach would use a HashSet<Character> rather than nested iterations of the list. You cut the runtime significantly at the cost of some memory. If you're not allowed to use a HashSet, an array of size 26 mapped to the letters of the alphabet would also work (assuming the input is constrained to all lowercase letters).

If you made all these changes, your code might look more like:

void removeDuplicatesInPlace(final Node head) {
    final Set<Character> listValues = new HashSet<>();
    Node current = head;
    listValues.add(current.val);

    while (current.next != null) {
        if (!listValues.add(current.next.val)) {
            current.next = current.next.next;
        } else {
            current = current.next;
        }
    }
}

class Node {
    final char val;
    Node next;

    Node(char val) {
        this.val = val;
    }

    @Override
    public String toString() {
        return Character.toString(val);
    }
}

public void removeDuplicatesInPlace() {
    final Node head = new Node('a');
    Node current = head;

    current.next = new Node('a');
    current = current.next;

    current.next = new Node('b');
    current = current.next;

    current.next = new Node('c');
    current = current.next;

    current.next = new Node('b');
    current = current.next;

    current.next = new Node('b');
    current = current.next;

    current.next = new Node('c');
    current = current.next;

    current.next = new Node('c');
    current = current.next;

    // a -> a -> b -> c -> b -> b -> c -> c
    this.removeDuplicatesInPlace(head);
    // a -> b -> c

    current = head;
    while (current != null) {
        System.out.print(current + " -> ");
        current = current.next;
    }
}

private static Node buildList(final char... values) {
    final Node head = new Node(values[0]);
    Node current = head;

    for (int i = 1; i < values.length; i++) {
        current.next = new Node(values[i]);
        current = current.next;
    }

    return head;
}


A basic C Shell for Linux I have written a basic C Shell. It features the builtins cd, programmer, ver and exit. The code for the shell, named vsh, is as follows:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/wait.h>

#define BUFSIZE 1024
#define TOK_BUFSIZE 64
#define TOK_DELIM " \t\r\n\a"

char **split_line(char *line)
{
    int bufsize = TOK_BUFSIZE, position = 0;
    char **tokens = malloc(bufsize * sizeof(char*));
    char *token, **tokens_backup;

    if (!tokens) {
        fprintf(stderr, "vsh: allocation error\n");
        exit(EXIT_FAILURE);
    }

    token = strtok(line, TOK_DELIM);
    while (token != NULL) {
        tokens[position] = token;
        position++;

        if (position >= bufsize) {
            bufsize += TOK_BUFSIZE;
            tokens_backup = tokens;
            tokens = realloc(tokens, bufsize * sizeof(char*));
            if (!tokens) {
                free(tokens_backup);
                fprintf(stderr, "vsh: allocation error\n");
                exit(EXIT_FAILURE);
            }
        }

        token = strtok(NULL, TOK_DELIM);
    }
    tokens[position] = NULL;

    return tokens;
}

int main(int argc, char **argv)
{
    int finish = 0;
    char *user = getenv("USER");
    char hostname[BUFSIZE];
    char *cwd;
    char line[BUFSIZE] = {0};
    char **args;
    char *directory;

    printf("Welcome to vsh\n");

    system("/bin/cat /etc/motd");

    printf("\n");

    gethostname(hostname, BUFSIZE);

    while (!finish)
    {
        cwd = getcwd(cwd, BUFSIZE);

        printf("[");
        printf(user);
        printf("@");
        printf(hostname);
        printf("] ");
        printf(cwd);
        printf(" $ ");
        fflush(stdout);

        if(NULL == fgets(line, sizeof line, stdin))
        {
            finish = 1;
            printf("\nLeaving vsh\n");
        }
        else
        {
            printf("The command read was %s", line);
            printf("\n");

            args = split_line(line);

            if(strcmp(line, "") == 0) {
                fprintf(stderr, "vsh: Expected command\n");
            }
            else if(strcmp(args[0], "cd") == 0)
            {
                directory = args[1];
                chdir(directory);
            }
            else if(strcmp(line,"programmer") == 0)
            {
                printf("The programmer of vsh is George Gibson\n");
            }
            else if(strcmp(line,"ver") == 0)
            {
                printf("The current version is 1.0\n");
            }
            else if(strcmp(line,"exit") == 0)
            {
                finish = 1;
                printf("\nLeaving vsh...\n");
                getchar();
            }
            else
            {
                pid_t pid;
                int err;

                pid = fork();

                if(pid == -1) {
                    perror("vsh");
                }
                else if(pid == 0) {
                    execvp(args[0], args);
                    perror("vsh");
                }
                else {
                    waitpid(pid, &err, 0);
                }
            }
        }
    }

    return 0;
}

I'm particularly looking for reviews relating to bugs, optimizations and general improvements.

In your main() function there is a while loop (while (!finish)) and in the first few lines of that there are a lot of printf() calls. Replace all of those printf's with this one line:

printf("[%s@%s] %s $ ", user, hostname, cwd);

It is easier to read that way, and with non optimising compilers is slightly more efficient.

Additionally, your main() function contains a lot of strings with vsh in them. I recommend you replace this:

printf("vsh: allocation error\n");

with this:

printf("%s: allocation error\n", argv[0]);

That will make sure that you can more easily change the name of your shell if you want to in the future. In functions other than main() the bother of passing argv[0] may be more than it is worth, though.


Counting business days in a month, ignoring holidays This table simply contains date information. I went ahead and used what I currently have since this is a one-time operation. However, since I only recently started working with Oracle, I would like to learn about all its different features and such.

The code is trying to establish if it is a business day or not while ignoring holidays. Essentially, we want a running total of business days for each day in a month. This value does need to be stored in the main table in the end.

Is there a better way to write this?

DECLARE
    Cursor MyCursor IS
        select  cc.calendar_dt,
                cc.CALENDAR_DT_ID,
                calendar_month_abbreviation,
                calendar_year_number,
                to_char(cc.calendar_dt,'MM') as m
        from    CALENDAR_DT cc
        order   by cc.calendar_dt ;

    entry MyCursor%ROWTYPE;

    new_number NUMBER := 1;
    curr_month NUMBER := 0;
begin

    FOR entry IN MyCursor LOOP

        if (curr_month != (entry.m)) then
            new_number := 1;
            curr_month := entry.m;
        else
            new_number := new_number + (case when to_char(entry.CALENDAR_DT,'D') in (1,7) then 0 else 1 end);
        end if;

        DBMS_OUTPUT.PUT_LINE(entry.calendar_dt || '    ' || new_number || '   ' || to_char(entry.CALENDAR_DT,'DAY') );

        --UPDATE CALENDAR_DT
        --        SET  business_day_number = new_number
        --        WHERE  CALENDAR_DT_ID = ENTRY.CALENDAR_DT_ID;

    END LOOP;

END;

Output with year=2014 and month in nov, dec and it is what I expect:

01-NOV-2014    1   SATURDAY
02-NOV-2014    1   SUNDAY
03-NOV-2014    2   MONDAY
04-NOV-2014    3   TUESDAY
05-NOV-2014    4   WEDNESDAY
06-NOV-2014    5   THURSDAY
07-NOV-2014    6   FRIDAY
08-NOV-2014    6   SATURDAY
09-NOV-2014    6   SUNDAY
10-NOV-2014    7   MONDAY
11-NOV-2014    8   TUESDAY
12-NOV-2014    9   WEDNESDAY
13-NOV-2014    10   THURSDAY
14-NOV-2014    11   FRIDAY
15-NOV-2014    11   SATURDAY
16-NOV-2014    11   SUNDAY
17-NOV-2014    12   MONDAY
18-NOV-2014    13   TUESDAY
19-NOV-2014    14   WEDNESDAY
20-NOV-2014    15   THURSDAY
21-NOV-2014    16   FRIDAY
22-NOV-2014    16   SATURDAY
23-NOV-2014    16   SUNDAY
24-NOV-2014    17   MONDAY
25-NOV-2014    18   TUESDAY
26-NOV-2014    19   WEDNESDAY
27-NOV-2014    20   THURSDAY
28-NOV-2014    21   FRIDAY
29-NOV-2014    21   SATURDAY
30-NOV-2014    21   SUNDAY
01-DEC-2014    1   MONDAY
02-DEC-2014    2   TUESDAY
03-DEC-2014    3   WEDNESDAY
04-DEC-2014    4   THURSDAY
05-DEC-2014    5   FRIDAY
06-DEC-2014    5   SATURDAY
07-DEC-2014    5   SUNDAY
08-DEC-2014    6   MONDAY
09-DEC-2014    7   TUESDAY
10-DEC-2014    8   WEDNESDAY
11-DEC-2014    9   THURSDAY
12-DEC-2014    10   FRIDAY
13-DEC-2014    10   SATURDAY
14-DEC-2014    10   SUNDAY
15-DEC-2014    11   MONDAY
16-DEC-2014    12   TUESDAY
17-DEC-2014    13   WEDNESDAY
18-DEC-2014    14   THURSDAY
19-DEC-2014    15   FRIDAY
20-DEC-2014    15   SATURDAY
21-DEC-2014    15   SUNDAY
22-DEC-2014    16   MONDAY
23-DEC-2014    17   TUESDAY
24-DEC-2014    18   WEDNESDAY
25-DEC-2014    19   THURSDAY
26-DEC-2014    20   FRIDAY
27-DEC-2014    20   SATURDAY
28-DEC-2014    20   SUNDAY
29-DEC-2014    21   MONDAY
30-DEC-2014    22   TUESDAY
31-DEC-2014    23   WEDNESDAY

Other than style changes (this was a really quick, one-time-only thing so sloppy doesn't truly matter, no one will ever see it but me and you guys - function does always matter)...

Is this approach the best (only?) one for this kind of operation in Oracle?

It is possible to do a cumulative sum in Oracle, indeed, using Analytic Function.

I've created a sample test table trying to reproduce the model presented in the question:

CREATE TABLE CALENDAR_DT AS
SELECT rownum calendar_dt_id
      ,DATE '2014-01-01'   + rownum - 1 calendar_dt
      ,0      business_day_number
FROM dual
CONNECT BY LEVEL <= 365;

A cumulative sum can be achieved using this sintax for instance:

SELECT SUM(t.calendar_dt_id) over(ORDER BY t.calendar_dt_id) cumulative_sum
      ,t.calendar_dt_id
FROM calendar_dt t;

The resultset will look like this

CUMULATIVE_SUM CALENDAR_DT_ID
-------------- --------------
         1              1
         3              2
         6              3
        10              4
        15              5
        21              6
        28              7
        36              8
        45              9
        55             10
        66             11
        78             12
        91             13
       105             14
       120             15
       136             16
       153             17
       171             18
       190             19
       210             20

The order by clause inside the parenthesis produces de cumulative sum.
If it is necessary to reset the sum every new month you can use the partition clause:

SELECT SUM(t.calendar_dt_id) over(PARTITION BY to_char(t.calendar_dt,'MM') ORDER BY t.calendar_dt_id) cumulative_sum
      ,t.calendar_dt
      ,t.calendar_dt_id
FROM calendar_dt t;

CUMULATIVE_SUM CALENDAR_DT CALENDAR_DT_ID
-------------- ----------- --------------
         1 1/1/2014                 1
         3 2/1/2014                 2
         6 3/1/2014                 3
        10 4/1/2014                 4
        15 5/1/2014                 5
        21 6/1/2014                 6
        28 7/1/2014                 7
        36 8/1/2014                 8
        45 9/1/2014                 9
        55 10/1/2014               10
        66 11/1/2014               11
        78 12/1/2014               12
        91 13/1/2014               13
       105 14/1/2014               14
       120 15/1/2014               15
       136 16/1/2014               16
       153 17/1/2014               17
       171 18/1/2014               18
       190 19/1/2014               19
       210 20/1/2014               20
       231 21/1/2014               21
       253 22/1/2014               22
       276 23/1/2014               23
       300 24/1/2014               24
       325 25/1/2014               25
       351 26/1/2014               26
       378 27/1/2014               27
       406 28/1/2014               28
       435 29/1/2014               29
       465 30/1/2014               30
       496 31/1/2014               31
        32 1/2/2014                32
        65 2/2/2014                33
        99 3/2/2014                34
       134 4/2/2014                35
       170 5/2/2014                36
       207 6/2/2014                37
       245 7/2/2014                38
       284 8/2/2014                39
       324 9/2/2014                40
       365 10/2/2014               41
       407 11/2/2014               42
       450 12/2/2014               43
       494 13/2/2014               44
       539 14/2/2014               45
       585 15/2/2014               46
       632 16/2/2014               47
       680 17/2/2014               48
       729 18/2/2014               49
       779 19/2/2014               50
       830 20/2/2014               51
       882 21/2/2014               52
       935 22/2/2014               53
       989 23/2/2014               54
      1044 24/2/2014               55
      1100 25/2/2014               56
      1157 26/2/2014               57
      1215 27/2/2014               58
      1274 28/2/2014               59
        60 1/3/2014                60

In those examples I used the id for summing just to get more evident. In your case you can use a sum of 1 conditioned to be a "day of week" using a Case expression exactly the way you used in your block, like this:

SELECT SUM(CASE
             WHEN to_char(t.calendar_dt, 'd') NOT IN (1, 7) THEN
               1
           END)
        over(PARTITION BY to_char(t.calendar_dt, 'MM')
             ORDER BY t.calendar_dt) new_business_day_number
      ,t.calendar_dt_id
      ,t.calendar_dt
      ,t.business_day_number
FROM calendar_dt t

After achieving the expected resultset, you can proceed with the update, but you don't need a block to do this if you use a MERGE statement:

MERGE INTO calendar_dt t
USING (SELECT SUM(CASE
                     WHEN to_char(t.calendar_dt, 'd') NOT IN (1, 7) THEN
                       1
                   END)
                over(PARTITION BY to_char(t.calendar_dt, 'MM')
                     ORDER BY t.calendar_dt) new_business_day_number
              ,t.rowid rid
        FROM calendar_dt t) u
ON    (u.rid = t.rowid)
WHEN  MATCHED THEN
  UPDATE
  SET    t.business_day_number = NVL(u.new_business_day_number,0);

I put the NVL function to avoid null value when the month happens to start in a weekend (feb-mar/2015 for instance) but I could have used the "ELSE 0" clause in the CASE expression as well.

I hope this helped you in some way.


HackersEarth - Reverse primes Generate as many distinct primes P such that reverse (P) is also prime
  and is not equal to P.

  Output: Print per line one integer( ≤ 10^15 ). Don't print more than
  10^6 integers in all.

  Scoring: Let N = correct outputs. M = incorrect outputs. Your score
  will be max(0,N-M).

  Note: Only one of P and reverse(P) will be counted as correct. If both
  are in the file, one will be counted as incorrect.

  Sample Output: 107 13 31 17 2

  Explanation

  Score will be 1. Since 13,107,17 are correct. 31 is incorrect because
  13 is already there. 2 is incorrect.

  Time Limit: 2 sec(s) for each input file. Memory Limit: 256 MB Source
  Limit: 25 KB

public class ReversePrime {

    final static int MAX = 1000000;
    final static int MAXLISTSIZE = 1000000;
    final static int BASE = 10;

    final static boolean[] isPrime = generatePrime();

    public static void main(String args[]) {
        Set<Long> reversedCheckedPrime = new LinkedHashSet<Long>();
        int isPrimeLength = isPrime.length;
        for (int i = 0; i < isPrimeLength; i++) {
            if (isPrime[i]) {
                long prime = 2 * i + 3;
                long reverse = reversePrime(prime);
                if ((!(prime == reverse))
                        && (!reversedCheckedPrime.contains(reverse))
                        && (reversedCheckedPrime.size() <= MAXLISTSIZE)) {
                    reversedCheckedPrime.add(prime);
                }
                if (reversedCheckedPrime.size() == MAXLISTSIZE) {
                    break;
                }
            }
        }

        for (Long prime : reversedCheckedPrime) {
            System.out.println(prime);
        }

    }

    private static long reversePrime(long prime) {
        long result = 0;
        long rem;
        while (prime != 0) {
            rem = prime % BASE;
            prime = prime / BASE;
            result = result * BASE + rem;
        }
        return result;
    }

    private static boolean[] generatePrime() {
        int root = (int) Math.sqrt(MAX) + 1;
        root = root / 2 - 1;
        int limit = (int) ((MAX - 1) / 2);
        boolean[] isPrime = new boolean[limit];
        Arrays.fill(isPrime, true);
        for (int i = 0; i < root; i++) {
            if (isPrime[i]) {
                for (int j = 2 * i * (i + 3) + 3, p = 2 * i + 3; j < limit; j = j
                        + p) {
                    isPrime[j] = false;
                }
            }

        }
        return isPrime;
    }
}

While the code is working the real concern is the time limit. I'm not able to get it below 2 seconds. How could I further improve the runtime?

One way I reduced it further by avoiding the println() call in the for loop by doing this:

StringBuilder outputResult = new StringBuilder();
        for (int i = 0; i < isPrimeLength; i++) {
            if (isPrime[i]) {
                long prime = 2 * i + 3;
                long revrse = reversePrime(prime);
                if ((!(prime == revrse))
                        && (!reversedCheckedPrime.contains(revrse))
                        && (reversedCheckedPrime.size() <= MAXLISTSIZE)) {
                    reversedCheckedPrime.add(prime);
                    outputResult.append(prime);
                    outputResult.append("\n");
                }
                if (reversedCheckedPrime.size() == MAXLISTSIZE) {
                    break;
                }
            }
        }

        System.out.println(outputResult);
    }

Question link for submission

Just a simple idea: From each pair, you generate both the smaller and the bigger prime. You're using a Set in order to assure that you don't output both. But this can be done much simpler:

Instead of

            long revrse = reversePrime(prime);
            if ((!(prime == revrse))
                    && (!reversedCheckedPrime.contains(revrse))
                    && (reversedCheckedPrime.size() <= MAXLISTSIZE)) {
                reversedCheckedPrime.add(prime);
            }

use

            long revrse = reversePrime(prime);
            if (prime < revrse) {
                reversedCheckedPrime.add(prime);
            }

You simply add the smaller one only (which also handles the case of -self-reversed numbers). The test

 reversedCheckedPrime.size() <= MAXLISTSIZE

was both wrong (should be <) and useless (as there's a break later).

Now you can replace the Set by a List and gain some speed. You can gain even more, if you use a long[] of length MAXLISTSIZE (which should be called MAX_LIST_SIZE) as you avoid autoboxing.


Using Python tricks to find Minimum Coin Change (Amazon Interview) What is the coin change problem?
The change-making problem addresses the question of finding the minimum number of coins (of certain denominations) that add up to a given amount of money. It is a special case of the integer knapsack problem and has applications wider than just currency. Read more: Wiki
My code:
def coin_change(n,coins,known_results):
     min_coins = n

     if n in coins:
        known_results[n] = 1
        return 1

     elif known_results[n] > 0:
            return known_results[n]

     else:
        for i in [c for c in coins if c<=n]:
            count = 1 + coin_change(n-i,coins,known_results)
            if count < min_coins:
                min_coins = count

                known_results[n] = min_coins

     return min_coins

coins = [1,2,3]
n = 4
known_results = [0]*(n+1)
print(coin_change(n,coins,known_results))

Question:
This code works fine and perfectly but can it be done better and more efficiently using python tricks giving it an edge more the other languages? Can it be more efficient and better?

Welcome to Code Review! Your code looks fine, and is fairly easy to follow. A few points to note though:
Since your initial if-elif clauses are returning values immediately, no need to wrap an else (and even elif) there:
if n in coins:
    known_results[n] = 1
    return 1
if known_results[n] > 0:
    return known_results[n]
for i in [c for c in coins if c<=n]:
    .
    .

is achieving the same thing.
In python, multiple assignments can be done in a single statement:
known_results[n] = min_coins = count

As an aside, you can make use of type hinting to make the values and parameters more understandable. If I was only reading through the function definition, I'd have no idea what known_results was supposed to be.


Arithmetic Progression I had to write a program that would complete an arithmetic progression based on the users input. The user would provide the third term in the series, the third from the last term, and the sum of all the terms in the series. The program would display the total number of terms are in the series and then the series itself. I already submitted it but I am hoping to get some general feedback.

Example: 3, 8, 55

Program output:

10

1, 2, 3, 4, 5, 6, 7, 8, 9, 10

#include <iostream>
#include <vector>

int main() {
    int thirdTerm;
    int thirdLastterm;
    int sum;
    int total;
    int constantNumber = 1;
    int a = 0;
    std::vector<int> vSeries;

    while (std::cout << "Please enter the third term in the series: " && !(std::cin >> thirdTerm)) {
        std::cin.clear(); //clear bad input flag
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); //discard input
        std::cout << "Invalid input; please re-enter.\n";
    }
    while (std::cout << "Enter the third from the last term in the series: " && !(std::cin >> thirdLastterm)) {
        std::cin.clear(); //clear bad input flag
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); //discard input
        std::cout << "Invalid input; please re-enter.\n";
    }
    while (std::cout << "Please enter the sum of the series: " && !(std::cin >> sum)) {
        std::cin.clear(); //clear bad input flag
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); //discard input
        std::cout << "Invalid input; please re-enter.\n";
    }

    do {
        a = 0;
        vSeries.clear();
        vSeries.push_back(thirdTerm - (2 * constantNumber));
        vSeries.push_back(thirdTerm - (1 * constantNumber));

        do {
            total = 0;
            vSeries.push_back(thirdTerm + (a * constantNumber));
            a++;
            for (int i = 0; i < vSeries.size(); i++) {
                total += vSeries[i];
            }
        } while (total < sum);

        constantNumber++;

        if (constantNumber > 100000) {
            std::cout << "There doesn't appear to be a solution based on the input - exiting program.";
                return 0;
        }

    } while (vSeries[vSeries.size() - 3] != thirdLastterm || total != sum);

    std::cout << "\nNumber of terms in the series: " <<  vSeries.size() << std::endl;
    for (int i = 0; i < vSeries.size(); i++) {
        if (i == 0)
            std::cout << vSeries[i];
        else
            std::cout << "," << vSeries[i];
    }

    return 0;
}

A couple of things I noticed:

Here:

while (std::cout << "Please enter the third term in the series: " && !(std::cin >> thirdTerm)) {
    std::cin.clear(); //clear bad input flag
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); //discard input
    std::cout << "Invalid input; please re-enter.\n";
}

Aside from the prompt string this is repeated three times.  This would be better if it was in a separate function that accepted the prompt string and the address of the variable to store the input value:

void GetInput(string prompt, int& input)
{
    while (std::cout << prompt && !(std::cin >> input))
    {
        std::cin.clear(); //clear bad input flag
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); //discard input
        std::cout << "Invalid input; please re-enter.\n";
    }
}

GetInput("Please enter the third term in the series: ",thirdTerm);
GetInput("Enter the third from the last term in the series: ",thirdLastterm);
GetInput("Please enter the sum of the series: ",sum);

Here:

    do {
        total = 0;
        vSeries.push_back(thirdTerm + (a * constantNumber));
        a++;
        for (int i = 0; i < vSeries.size(); i++) {
            total += vSeries[i];
        }
    } while (total < sum);

You're iterating through the vector getting a new total.  It would be better to just add each element to a running total:

    total = 0;
    do {
        int temp = thirdTerm + (a * constantNumber);
        vSeries.push_back(temp);
        total += temp;
        a++;
    } while (total < sum);


Generating filesystem paths from a fixed string It's clever, but makes me vomit a little:

file = '0123456789abcdef123'

path = os.sep.join([ file[ x:(x+2) ] for x in range(0,5,2) ])

Is there are reason you're not just doing:

path = os.sep.join([file[0:2], file[2:4], file[4:6]])

I think that my version is a little easier to parse (as a human), but if you need to extend the number of groups, your code is more extensible.

Edit: and if we're looking for things that are easy to read but not necessarily the best way to do it...

slash = os.sep
path = file[0:2] + slash + file[2:4] + slash + file[4:6]


Wait for System.IO.StreamReader.ReadLineAsync I want to wait for a line to be read, but only for so long before timing out.  This is what I came up with.  Is there a better way to do it?

Dim reader As New System.IO.StreamReader(pipe)

Dim nextCommand = Await New Func(Of Task(Of String))(
    Function()
        Dim t = reader.ReadLineAsync()
        If (Not t.Wait(2000)) Then Throw New MyTimeoutExeption()
        Return t
    End Function).Invoke()

For operations that don't support cancellation themselves, You can combine Task.WhenAny() with Task.Delay():

Async Function TryAwait(Of T)(target As Task(Of T), delay as Integer) As Task(Of T)
    Dim completed = Await Task.WhenAny(target, Task.Delay(delay))
    If completed Is target Then
        Return Await target
    End If

    Throw New TimeoutException()
End Function

Usage:

Dim nextCommand = Await TryAwait(reader.ReadLineAsync(), 2000)


Checking if an array of dates are within a date range I created a dates_in_range() function which checks if all of the dates passed to it ($dates array) are within $start_date and $end_date. If they aren't, it'll return false.
The code works fine, but I'm wondering if there's a way to refactor it better using something like array_filter? Or does this look fine?
function dates_in_range( $start_date, $end_date, $dates = array() ) {

    foreach ( $dates as $date ) {
        if ( ! ( ( (strtotime( $date ) >= strtotime( $start_date ) ) && ( strtotime( $date ) <= strtotime( $end_date ) ) ) ) ) {
            return false;
        }
    }

    return true;
}

$dates = array(
    '2021-11-19 00:00:00',
    '2020-11-20 00:00:00',
);

$start_date = '2021-10-01 00:00:00';
$end_date   = '2021-12-31 00:00:00';

// Returns true
if ( dates_in_range( $start_date, $end_date, $dates ) ) {
    echo "In range.";
} else {
    echo "Not in range.";
}

To me it looks fine, other than it makes me scroll the code to read it. On this ground I agree with Sam, that line ought to be shortened. To do so I would

use the fact that you don't need to convert mysql format dates to compare them
remove excessive bracing
use De Morgan's law to invert the condition

and get this
function dates_in_range( string $start_date, string $end_date, array $dates ): bool
{
    foreach ( $dates as $date ) {
        if ( $date < $start_date || $date > $end_date ) {
            return false;
        }
    }
    return true;
}

Also, I removed the default value for the $dates as I don't see any use in calling this function with only two parameters.
I am not sure about empty arrays though. Your function will return true for one, but it's hard to tell whether it's correct or not. It's like division by zero, and may be an exception must be thrown.


Run an external program and extract a pattern match along with the result file The script takes two input files of protein sequences and runs an external program (installed in linux/MacOS). The result provides a text output file example output.. Identity percentage is extracted from the matched keyword "Identity" in the line. Also, the script returns the result text file.

Can you help me to improve this script?

import re
import subprocess

def cmdline(command):
    process = subprocess.Popen(
        args=command,
        stdout=subprocess.PIPE,
        shell=True
    )
    return process.communicate()[0]

def blast_two_sequences(file1, file2):

    try:
        # fetch the identity percentage
        #https://www.biostars.org/p/200256/

        cmd = 'needle -datafile EBLOSUM62 -auto Y' + ' -asequence ' + file1 +' -bsequence ' + file2 +  ' -sprotein1 Y -sprotein2 Y ' + ' -auto -stdout'
        # cmd = 'needle -asequence ' + file1 +' -bsequence ' + file2 + ' -auto -stdout'

        results = cmdline(cmd)

        results = results.decode("utf-8")

        identity = re.search(r"\d{1,3}\.\d*\%", results)
        identity = identity.group()
        identity = identity.replace('%', '')

        return identity, results

    except:
        return None

file1 = "/Users/catuf/Desktop/file1.txt"
file2 = "/Users/catuf/Desktop/file2.txt"

if blast_two_sequences(file1, file2) is not None:
    identity, result = blast_two_sequences(file1, file2)

Not looking bad for as far as I can see. If the example file is accurate for the lengths of the input files, then I don't forsee any real problems, though others may of course disagree.

Naming:

cmdline is quite a... short name for a function. I'd think cmd_line for snake_case convention. However, what it does is create what's basically a text file in a bytes buffer by means of running a program. While you could call that a command line execution if you're working from a terminal, I'd name it after the invoked program, or something more generic like get_input_data. Since generic is always survivable at best, I'd pick something like get_needle_results or something like that.

For a module like subprocess which you can guess does creative things dependent on what platform you're on, it's always advisable to grab the docs:

  args should be a sequence of program arguments or else a single string. By default, the program to execute is the first item in args if args is a sequence. If args is a string, the interpretation is platform-dependent and described below. See the shell and executable arguments for additional differences from the default behavior. Unless otherwise stated, it is recommended to pass args as a sequence.

And your platform matters - a lot. If you never switch platform or needle implementation, then you can do what you're doing, but if not you're risking a collossal headache with potentially giving it wrong arguments. Therefore, you might want to restructure it to a list of arguments.

And if you don't, use f-strings, the best invention since the list comprehension:

cmd = 'needle -datafile EBLOSUM62 -auto Y' + ' -asequence ' + file1 +' -bsequence ' + file2 +  ' -sprotein1 Y -sprotein2 Y ' + ' -auto -stdout'
# Becomes:
cmd = f'needle -datafile EBLOSUM62 -auto Y -asequence {file1} -bsequence {file2} -sprotein1 Y -sprotein2 Y  -auto -stdout'

Which is shorter and more readable.

        results = cmdline(cmd)

        results = results.decode("utf-8")

These can be combined:

        results = cmdline(cmd).decode("utf-8")

So can these:

        identity = re.search(r"\d{1,3}\.\d*\%", results)
        identity = identity.group()
        identity = identity.replace('%', '')
# New:
        identity = re.search(r"\d{1,3}\.\d*\%", results).group().replace('%', '')

if blast_two_sequences(file1, file2) is not None:
    identity, result = blast_two_sequences(file1, file2)

And ouch... you're actually running your entire program twice. If your needle executions are counted, you might have noticed this. You should refactor this to:

output = blast_two_sequences(file1, file2)
if output is not None:
    identity, result = output
# Or if you work with Python 3.8+
if output := blast_two_sequences(file1, file2) is not None:
    identity, result = output

You should also put a guard around the actual execution, so that a later scripter can import your functions and use them without executing your script on import:

if __name__ == "__main__":
    file1 = "/Users/catuf/Desktop/file1.txt"
    file2 = "/Users/catuf/Desktop/file2.txt"
    if output := blast_two_sequences(file1, file2) is not None:
        identity, result = output

Exceptions:

There's a small issue you have - you have a bare except in your code. That means that if weird stuff happens, your program will just ignore it and return none. In your case, I would recommend removing the try: statement completely, and you'll be informed about any problems that arise. If you're watching out for specific Exceptions, narrow your except: to those, and preferably restrict your try/except to directly around the origin lines of these Exceptions.

This will obsolete your None check on the outside. Exceptions showing up on your screen is a good thing - it means you'll be notified when the unexpected happens and you have some unknown error - because now you can go fix it.


Reversing a String using Stack Is this code okay?

import java.util.Stack;

public class StackReverse {

    public static void main(String[] args) {

        final String inputString = "code review";

        final String reversed = reverseString(inputString);
        System.out.println("The reversed string is " + reversed);

    }

    public static String reverseString(String originalString) {
        Stack<Character> stack = new Stack<>();
        String reversed = "";
        for (int i = 0; i < originalString.length(); i++) {
            char ch = originalString.charAt(i);
            stack.push(ch);
        }
        for (int i = 0; i < originalString.length(); i++) {
            char ch = stack.pop();
            reversed = reversed + ch;

        }
        return reversed;

    }

}

Unicode is hard to get right, especially in Java as it has a more or less broken concept of a “character”. An Unicode code point is a 21-bit number. These code points are encoded to bytes with the UTF-8 or UTF-16 encodings (well, there are a couple more…). But when Java was created, Unicode only had characters in a 16-bit range, and code points were encoded with the (now deprecated) UCS-2 encoding.

There's a lot of pain from the fact that the 21-bit Unicode code points don't fit into a single 16-bit char or Character in Java. Two consecutive chars might actually be surrogate pairs. When reversing a string, we have to special-case these. We can use the Character.isHighSurrogate(char) function to test whether a given char starts a surrogate pair. If we encounter such a code point, we advance to the next char in the string, and push it onto the stack first:

for (int i = 0; i < originalString.length(); i++) {
    char ch = originalString.charAt(i);
    if (Character.isHighSurrogate(ch)) {
        i++;
        if (i < originalString.length()) {
            stack.push(originalString.charAt(i));
        }
    }
    stack.push(ch);
}

As a test case, you can reverse a string with the smiley character : "hi \ud83d\ude03". Reversed, it should be "\ud83d\ude03 ih".

It is best to think of Java's char as “an UTF-16 code unit” (see Wikipedia for all the details about UTF-16 you didn't want to know, but should). To learn about what Unicode is, and how to tame it, start with Joel Spolsky's Unicode blog post.


Checking whether users have a role that allows them to index sheets Is there any DRYer way to write this method ? or is it better to keep it this way for better reader understanding ...

Each role has a list of authorised activities for a specific resource ( in this case Sheet) , a user may have multiple roles ( depending upon his company, his working group in the company, his function in the working group). This method is a sheet_policy, if among all roles , the current user can "index" sheets, the method returns true otherwise it returns false

   def index?
    # check if sheet#index is listed in current_user roles activities
    sheet_activities = []
    user.roles.each {|role| sheet_activities << role.activities["sheet"]}
    return false if sheet_activities.first.nil? # no authorized activities on Sheet
    sheet_activities.flatten.uniq!.include?("index")
  end

activities is a JSONB attribute of the Sheet record ...

create_table :roles do |t|
  t.string      :company, null: false, default: Settings.company.default.name
  t.string      :group
  t.string      :function
  t.jsonb       :activities, null: false, default: '{}'
  t.timestamps  null: false

Seeding example :

   @role_accounting_employee = Role.create( group: "accounting", function: "employee")  # default company : "internal"
   @role_accounting_employee.activities.deep_merge!({sheet: %w(show index) })

[Improved with flat_map thanks to @tokland.]

How about:

def index?
  user.roles.flat_map { |role| role.activities['sheet'] }.include?('index')
end

In Ruby, it is almost never necessary to set a temporary variable to empty and iterate over an object, given the power of methods like map, reduce, find, all?, etc. I don't see the need to use uniq! since it doesn't change whether index will be present or not. It's unclear why you're checking for nil on the first sheet_activities (since you can't guarantee what will be first without sorting), but if you just want to return false when sheet_activities are nil, the above will already do so.

Also note that Rubocop likes single quotes when you're not interpolating.


A .py utility file for neural network learing rate policies I've created a .py utility file, which specifies the learning rate policy for the neural network in PyTorch. The program in prior reads a json and creates a dict, called here is cfg.
So far, I have implemented 3 kinds of learning policies - steps, exp, cos, and for each of them I've created a separate class with the __call__ method. I've done this to have this parameters set in the LambdaLR without further refering to cfg during the training.
In order to avoid branch of if's I've created a dict, which chooses one of the options, and in the function create_scheduler the instance of one of these classes is created, passed to the initializer of LambdaLR scheduler, and this scheduler is then returned.
class LR_Steps:
    def __init__(self, cfg):
        self.lr_base = cfg["OPTIM"]["BASE_LR"]
        self.lr_mult = cfg["OPTIM"]["LR_MULT"]
        self.steps   =  cfg["OPTIM"]["STEPS"]
        # the current index in the steps
        self.ind = 0

    def __call__(self, epoch):
        if epoch > self.steps[self.ind]:
            self.ind += 1

        return self.lr_base * (self.lr_mult ** self.ind)

class LR_Exp:
    def __init__(self, cfg):
        self.lr_base = cfg["OPTIM"]["BASE_LR"]
        self.lr_mult = cfg["OPTIM"]["LR_MULT"]

    def __call__(self, epoch):
        return self.lr_base * (self.lr_mult ** epoch)

class LR_Cos:
    def __init__(self, cfg):
        self.lr_base = cfg["OPTIM"]["BASE_LR"]
        self.max_epoch = cfg["OPTIM"]["MAX_EPOCH"]

    def __call__(self, epoch):
        return 0.5 * self.lr_base * (1.0 + np.cos(np.pi * epoch / self.max_epoch))

lr_policy_dict = {
    "cos" : LR_Cos,
    "exp" : LR_Exp,
    "steps" : LR_Steps
}

def create_scheduler(optimizer, cfg):
    return optim.lr_scheduler.LambdaLR(optimizer, lr_policy_dict[cfg["OPTIM"]["LR_POLICY"]](cfg))

My question is - if there a shorter or safer way to do this? I would also appreciate criticism, concerning the structure of code and functionality.

Overall I rather like this. It's a nice way to think about "configurable functions". Minor stuff:

Due to order of operations, the parens around self.lr_base * (self.lr_mult ** epoch) are not needed
It's worth splitting out a temporary variable for cfg['OPTIM']
Consider annotating your member variables and method signatures with PEP484 type hints
Decimals in 1.0 are not necessary due to float promotion
Spaces before colons - "cos" : - are not PEP8-standard
You've not shown unit tests. If you don't have them, write them.

If you're interested, a different (not necessarily better) way of thinking about these functions is that they can start as normal, non-class functions, where the first parameter(s) are the configuration needed; then call partial() to get a callable reference that you can then use as if it has only epoch as a parameter from the perspective of  LambdaLR.


Checking for required text fields for login I wrote this script and it's working quite well however I think there's too much if else statements on it. How could I change them for a switch statement?

@IBAction func sendLoginParameters(_ sender: UIButton) {

    if correoTextField.text?.isEmpty == true && contrasenaTextField.text?.isEmpty == true {
        let alertMessage = UIAlertController(title: "Datos requeridos", message: "Favor de llenar todos los datos requeridos (CORREO Y CONTRASEÑA) antes de continuar", preferredStyle: .alert)
        alertMessage.addAction(UIAlertAction(title: "Aceptar", style: .default, handler: nil))
        self.present(alertMessage, animated: true, completion: nil)
        return

    } else if correoTextField.text?.isEmpty == true  {

        let alertMessage = UIAlertController(title: "Datos requeridos", message: "Favor de llenar todos los datos requeridos (CORREO) antes de continuar", preferredStyle: .alert)
        alertMessage.addAction(UIAlertAction(title: "Aceptar", style: .default, handler: nil))
        self.present(alertMessage, animated: true, completion: nil)
        return

    } else if contrasenaTextField.text?.isEmpty == true {

        let alertMessage = UIAlertController(title: "Datos requeridos", message: "Favor de llenar todos los datos requeridos (CONTRASEÑA) antes de continuar", preferredStyle: .alert)
        alertMessage.addAction(UIAlertAction(title: "Aceptar", style: .default, handler: nil))
        self.present(alertMessage, animated: true, completion: nil)
        return
} else {
...

In my opinion the issue here is not the repetition of if else but the repetition of similar alert message code.

For example, you could continue using if else but without these repetitions:

guard let correoText = correoTextField.text, let contrasenaText = contrasenaTextField.text else {
    fatalError("The outlets are not properly connected")
}
var msg: String? = nil
if correoText.isEmpty && contrasenaText.isEmpty {
    msg = "CORREO Y CONTRASEÑA"
} else if correoText.isEmpty {
    msg = "CORREO"
} else if contrasenaText.isEmpty {
    msg = "CONTRASEÑA"
}
if let msg = msg {
    let alertMessage = UIAlertController(title: "Datos requeridos", message: "Favor de llenar todos los datos requeridos (\(msg)) antes de continuar", preferredStyle: .alert)
    alertMessage.addAction(UIAlertAction(title: "Aceptar", style: .default, handler: nil))
    self.present(alertMessage, animated: true, completion: nil)
}

But if you want to use switch, you can switch on a tuple representing the two booleans:

guard let correoText = correoTextField.text, let contrasenaText = contrasenaTextField.text else {
    fatalError("The outlets are not properly connected")
}
let msg: String?
switch (correoText.isEmpty, contrasenaText.isEmpty)  {
case (true, true):
    msg = "CORREO Y CONTRASEÑA"
case (true, false):
    msg = "CORREO"
case (false, true):
    msg = "CONTRASEÑA"
default:
    msg = nil
    // implement additional behavior, or add more cases
}
let alertMessage = UIAlertController(title: "Datos requeridos", message: "Favor de llenar todos los datos requeridos (\(msg)) antes de continuar", preferredStyle: .alert)
alertMessage.addAction(UIAlertAction(title: "Aceptar", style: .default, handler: nil))
self.present(alertMessage, animated: true, completion: nil)


Editing system files in Linux (as root) with GUI and CLI text editors #2 (final) This is the second iteration of this question

A note to start with: the first iteration of this question can be found here:
Editing system files in Linux (as root) with GUI and CLI text editors

As stated in there:

  My intention is to POSIX-ly write one generalized function for running various text editors I use for different purposes through sudoedit, i.e. editing files as root safely. Safely = for instance, if a power loss occurs during the file edit; another example could be lost SSH connection, etc.

I come to you now with a possible final solution:

sudoedit enhanced

# USAGE: Just source this file into your shell for instance if using Bash, then you can use these files: ~/.bashrc or ~/.bash_aliases

# Please, customize these lists to your preference before using this script!
sudoedit__cli_editor_list='nano vi'
sudoedit__gui_editor_list='gedit emacs xed subl' # VS Code has its own work-around

sudoedit_enhanced_run ()
# Generic function for text editing as root
# the proper, safe, way through `sudoedit`.
# Expected arguments:
# $1 = editor name; obviously mandatory
# $2 = wait option; to avoid this, pass an empty string ('')
# $3, ($4), ... = file(s); at least one file must be given
{
    # check the minimum number of arguments
    if [ $# -lt 3 ]; then
        printf '%b\n' "sudoedit_enhanced_run(): Low number of arguments.\\nExpected: \$1 = editor name; \$2 = wait option; \$3, (\$4), ... = file(s).\\nPassed $#: $*" >&2
        return 1
    fi

    # let's take a closer look at the first argument, the editor
    editor_name=$1

    # store an editor alias, if there is any
    editor_alias=$( alias "$editor_name" 2> /dev/null )

    # remove that alias for now
    if [ -n "$editor_alias" ]; then
        unalias "$editor_name"
    fi

    # find out if such editor exists on the system
    # and store the first two arguments to variables
    editor_path=$( command -v "$editor_name" 2> /dev/null )
    wait_option=$2

    # if that editor does not return valid path, print error and bail
    if ! [ -x "$editor_path" ]; then
        printf '%s\n' "sudoedit_enhanced_run(): This editor ('$editor_name') is not installed on this system." >&2
        return 1
    fi

    # if we got here, then both of the things are ok;
    # so let's move past the editor and its wait option to the actual files
    shift 2

    # check if all the files exist, it does not make sense to create a file this way
    for file in "$@"; do
        if ! [ -f "$file" ]; then
            printf '%s\n' "sudoedit_enhanced_run(): This file ('$file') does not exist or it is not a regular file." >&2
            return 1
        fi
    done

    # run the editor with one-time SUDO_EDITOR set-up
    SUDO_EDITOR="$editor_path $wait_option" sudoedit "$@"

    # re-define the editor alias, if there was any, afterward
    if [ -n "$editor_alias" ]; then
        eval "$editor_alias"
    fi
}

# Editor aliases generators / definitions

for cli_editor in $sudoedit__cli_editor_list; do
    alias su$cli_editor="sudoedit_enhanced_run $cli_editor ''"
done

for gui_editor in $sudoedit__gui_editor_list; do
    alias su$gui_editor="sudoedit_enhanced_run $gui_editor -w"
done

# VS Code specific workaround to work under root
alias sucode="sudo mkdir -p /root/.vscode && sudo code -w --user-data-dir=/root/.vscode"

I would like to get a second opinion before posting it to my GitHub.

Already posted on GitHub, whereas I also tried hard to describe the purpose and (in spite it shall be obvious) usage including some example images on the project GitHub page.

Self-review

comments

Comments are very important to future readers, they speed up the comprehension of the whole code. I think I messed up at least one comment, (others pending review):

original:

# let's take a closer look at the first argument, the editor

suggested:

# store the first argument, the editor name

combine what can be combined

By combining simple pieces of code, we make it easier to read.

original:

# store an editor alias, if there is any
editor_alias=$( alias "$editor_name" 2> /dev/null )

# remove that alias for now
if [ -n "$editor_alias" ]; then
    unalias "$editor_name"
fi

suggested:

# store an editor alias; and if there is any, remove it for now
if editor_alias=$( alias "$editor_name" 2> /dev/null ); then
    unalias "$editor_name"
fi

implement code workaround into the function

My previous solution does not do any checks for code and also, by doing this we get rid of that alien alias.

original:

# VS Code specific workaround to work under root
alias sucode="sudo mkdir -p /root/.vscode && sudo code -w --user-data-dir=/root/.vscode"

suggested:

# run the editor with one-time SUDO_EDITOR set-up
if [ "$editor_name" = code ]; then
    # code specific workaround
    sudo mkdir -p /root/.vscode &&
    sudo code -w --user-data-dir=/root/.vscode "$@"
else
    # main command generic
    SUDO_EDITOR="$editor_path $wait_option" sudoedit "$@"
fi

avoid generating editors aliases for which editor is not installed

My previous solution is generating all editor aliases, no matter if such program is installed on the system, this could have been unpleasant to users.

original:

for cli_editor in $sudoedit__cli_editor_list; do
    alias su$cli_editor="sudoedit_enhanced_run $cli_editor ''"
done
for gui_editor in $sudoedit__gui_editor_list; do
    alias su$gui_editor="sudoedit_enhanced_run $gui_editor -w"
done

suggested:

for cli_editor in $sudoedit__cli_editor_list; do
    if command -v "$cli_editor" > /dev/null 2>&1; then
        alias su$cli_editor="sudoedit_enhanced_run $cli_editor ''"
    fi
done
for gui_editor in $sudoedit__gui_editor_list; do
    if command -v "$gui_editor" > /dev/null 2>&1; then
        alias su$gui_editor="sudoedit_enhanced_run $gui_editor -w"
    fi
done


Printing/Saving a Payroll Chart This code was inspired by this question: Payroll Program.  I saw lots of ways I thought the code could be improved.  So much so that I thought it wouldn't be particularly constructive to post all of it as an answer to that question.  Essentially, I wanted to basically rewrite the whole thing from scratch.

So I did.  But I'm far from a C++ expert, so tell me how to make this even better:

#include <cstdlib>
#include <iostream>
#include <fstream>
#include <string>
#include <iomanip>
#include <vector>
#include <sstream>

const double WORK_WEEK_HOURS = 40;
const double OVERTIME_RATE = 1.5;

struct employee {
    int id;
    double payrate;
    double hours;
};

void payroll();
std::string fetchFileName();
employee fetchEmployee();
double wagesForEmployee(employee e);
bool askUser(std::string s);
std::string employeesToString(std::vector<employee> e);
void displayEmployees(std::vector<employee> e);
void saveEmployeesToFile(std::vector<employee> e);

int main(int argc, const char * argv[]) {
    payroll();
    return 0;
}

void payroll() {
    std::vector<employee> employees;

    do {
        employees.push_back(fetchEmployee());
    } while (askUser("Would you like to enter more employees? (y/n): "));

    if (askUser("Would you like to view the information you entered? (y/n): ")) {
        displayEmployees(employees);
    }

    if (askUser("Would you like to save the information you entered to file? (y/n): ")) {
        saveEmployeesToFile(employees);
    }
}

bool askUser(std::string s) {
    char answer;
    std::cout << s;
    std::cin >> answer;
    return (answer == 'y' || answer == 'Y');
}

employee fetchEmployee() {
    employee e;
    e.id = -1;
    e.payrate = -1;
    e.hours = -1;

    do {
        std::cout << "Enter the employee ID: ";
        std::cin >> e.id;
    } while(e.id < 0);

    do {
        std::cout << "Enter the payrate: ";
        std::cin >> e.payrate;
    } while (e.payrate < 0);

    do {
        std::cout << "Enter the hours worked: ";
        std::cin >> e.hours;
    } while (e.hours < 0);

    return e;
}

double wagesForEmployee(employee e) {
    if (e.hours < 0 || e.payrate < 0) {
        return 0;
    }
    double wages = 0;
    if (e.hours > WORK_WEEK_HOURS) {
        wages += (e.hours - WORK_WEEK_HOURS) * OVERTIME_RATE * e.payrate;
    }
    wages += std::min(e.hours, WORK_WEEK_HOURS) * e.payrate;
    return wages > 0 ? wages : 0;
}

std::string employeesToString(std::vector<employee> e) {
    std::stringstream ss;

    ss << std::setw(10) << "ID" << std::setw(10) << "Rate" << std::setw(10) << "Hours" << std::setw(10) << "Wage" << std::endl;
    ss << "---------------------------------------------------------" << std::endl;

    for (int i = 0; i < e.size(); ++i) {
        employee curEmp = e.at(i);
        ss  << std::setw(10) << curEmp.id << std::setprecision(2) << std::fixed
            << std::setw(10) << curEmp.payrate << std::setprecision(2) << std::fixed
            << std::setw(10) << curEmp.hours << std::setprecision(2) << std::fixed
            << std::setw(10) <<  wagesForEmployee(curEmp) << std::endl;
    }

    return ss.str();
}

void displayEmployees(std::vector<employee> e) {
    std::cout << employeesToString(e) << std::endl;
}

void saveEmployeesToFile(std::vector<employee> e) {
    std::ofstream outputFile;
    std::string fileName;

    std::cout << "Enter a file name: ";
    // TODO: Safety checking for illegal file name characters, etc.
    std::cin >> fileName;
    fileName += ".pay";
    outputFile.open(fileName.c_str());
    outputFile << employeesToString(e);
    outputFile.close();

    std::cout << "Successfully saved to " << fileName;
}

double wagesForEmployee(employee e);
bool askUser(std::string s);
std::string employeesToString(std::vector<employee> e);
void displayEmployees(std::vector<employee> e);
void saveEmployeesToFile(std::vector<employee> e);

When you're only reading from a variable and not modifying it, you should take it by const reference. Taking it by value like this means that you copy it.

This is particularly bad in displayEmployees since it makes a copy of the entire employee vector then passing that copy to employeesToString which then makes another copy.

e.id = -1;
e.payrate = -1;
e.hours = -1;

do {
    std::cout << "Enter the employee ID: ";
    std::cin >> e.id;
} while(e.id < 0);

Rather interestingly, you've manged to stumble across a backwards-compatibility breaking change in C++11. As per cppreference:

Before C++11:

  If extraction fails (e.g. if a letter was entered where a digit is expected), value is left unmodified and failbit is set.

Since C++11:

  If extraction fails, zero is written to value and failbit is set.

This means that compiled under C++11 or newer, this code is wrong. A better approach is to simply check the result of the extraction directly:

do {
    std::cout << "Enter the employee ID: ";
} while(!(std::cin >> e.id));

Note that this has the behavior of checking whitespace delimited words. In other words, if you want to skip the rest of the line on erroneous input, you'll need to do something likely involving std::getline to eat the rest of the line. As it is, entering something like a b c 5 would output the prompt text 4 times but only actually allow the user to provide input once. (And it would exit the loop with e.id == 5.)

int main(int argc, const char * argv[]) {
    payroll();
    return 0;
}

I'd probably just pull payroll up into main since main doesn't do anything else.
Omitting return 0 silently inserts return 0. In other words, when a program only exits with a success status, you can omit the return statement so that people reading the code can see that at a glance.
If you don't use argc and argv, I wouldn't bother to take them. int main() is just as valid.

double wagesForEmployee(employee e) {
    // #1
    if (e.hours < 0 || e.payrate < 0) {
        return 0;
    }
    double wages = 0;
    // #2
    if (e.hours > WORK_WEEK_HOURS) {
        wages += (e.hours - WORK_WEEK_HOURS) * OVERTIME_RATE * e.payrate;
    }
    // #2
    wages += std::min(e.hours, WORK_WEEK_HOURS) * e.payrate;
    // #3
    return wages > 0 ? wages : 0;
}

If you're going to start applying business rules to employee, I'd put them in the employee class. This would unfortunately complicate complicate the code to prompt for employee information on the command line though.
Super minor and completely opinion: I would calculate normal wages and then add overtime on top. It threw me for a second to realize that overtime was being done first.
When wages >= 0, this is a (logical) no-op. That means that you're verifying that wages are positive. But, earlier in the function, you've verified that e.hours is nonnegative and payrate is nonnegative. OVERTIME_RATE is also presuambly nonnegative. In other words, why is this check here?

As a general note: don't hide error conditions. 0 is presumably a valid return from this function, so using it as an error sentinel is throwing away information. Instead, you need to either assert or throw an exception when you get unexpected inputs. Which you do depends on if you consider invalid inputs programmer error or user error. Personally, I would use assert here since I would expect the consuming programmer to provide a sane employee. (Actually I wouldn't check it at all, but some might say I err too far on the side of anti-precondition-verification.)

Another general note: this is a function that is oriented around an employee instance. It's also deeply aware of the internals of an employee object, and it's coupled to it. You see where I'm going with this, but just in case: this should be a method on the employee class. Stop seeing employee as a data bag, and start seeing it as a class. The e.hours >= 0 and e.payrate >= 0 conditions can be handled on the class, and this method would fit naturally onto it. You could even implement operator<<(ostream&) on it to simply printing it (though personally I have mixed feelings on human aimed operator<<(ostream&) implementations).

With regards to your employee outputting, you should have a function to output a single employee and then loop over that.

Also, Converting employees to a string and then outputting that string is incredibly inefficient. A better approach in C++ than building strings and outputting them is to output to streams and then build a string from that if you so desire. For example:

std::ostream& outputEmployee(ostream& os, const employee& e) {

    // You would of course want to do proper formatting -- just a lazy example.
    // Note that a new line is not included. That is because the newline implies some form of looping is happening, but this function operates on a single employee.
    os << e.id << ' ' << e.payrate << ' ' << e.hours;

    return os; // So the caller can easily check success ala if (outputEmployee(os, emp)) { ... }

    // In a real function, I would be tempted to put the return in the same line as the stream writing.
    // That has the same effect, but it saves a useless `return os;` line.
    // Example: return os << e.id << ... ;

}

std::string employeeToString(const employee& e) {
    std::ostringstream oss;
    // If you wanted to be really paranoid, you could check oss after this call.
    // Nothing should (reasonably) fail though, so meh.
    outputEmployee(oss);
    return oss.str();
}

This is pretty much the exact same process you wrote, but the overhead of buffering into a string only to dump out to a stream is avoided.

As mentioned earlier, you could override operator<<(ostream&). That would have the advantage of being a tiny bit cleaner to call (std::cout << emp << '\n'), and interestingly enough, it would also allow you to use some standard library tricks. For example, if you wanted to loop over outputting a collection of employees:

std::copy(std::begin(employees), std::end(employees),
          std::ostream_iterator<employee>(std::cout, "\n"));

Reading employees in can be done in a similar way if operator>> is overloaded:

std::vector<employee> employees;
std::copy(std::istream_iterator<employee>(std::cout),
          std::istream_iterator<employee>(),
          employees);

As mentioned in passing earlier though, human aimed >> and << make me a little uncomfortable (at least when the operators are defined directly on the class and not some kind of formatting decorator). I more of see them as debugging methods or methods for automatic reading (like parsing in a file or something instead of prompting a user).

I don't like how much saveEmployeesToFile does. I would pull the prompting for file name and opening of the file into at least 1 function and have saveEmployeesToFile take an ostream&. Happy side effect: you can then easily support outputting to std::cout if you want to pass that instead of a file stream.

With regards to saveEmployeesToFile:

Provided that functions stay small and focused, std::ofstream::close() is mostly noise. The file will be closed on destruction anyway.
If you prompt for the file name before you define the std::ofstream instance, you can avoid the open call.
You should verify that the file was successfully opened.

#includes should be alphabetized (though grouped by source first -- local includes first then 3rd party libraries and the standard library).


Is this a good approach for creating pages for multiple content typs with GatsbyJS? I'm trying to create pages for multiple content types in Gatsby and I was wondering if this is the right approach:

My gatsby-node.js file:

const path = require(`path`)

const { createFilePath } = require("gatsby-source-filesystem")
exports.onCreateNode = ({ node, actions, getNode }) => {
  const { createNodeField } = actions
  // Is this OK?
  if (
    node.internal.type === "Mdx" &&
    node.fileAbsolutePath.includes(`content/posts`)
  ) {
    const value = createFilePath({ node, getNode })
    createNodeField({
      name: "slug",
      node,
      value: `/blog${value}`,
    })
  }
  if (
    node.internal.type === "Mdx" &&
    node.fileAbsolutePath.includes(`content/projects`)
  ) {
    const value = createFilePath({ node, getNode })
    createNodeField({
      name: "slug",
      node,
      value: `/project${value}`,
    })
  }
}
exports.createPages = async ({ graphql, actions, reporter }) => {
  const { createPage } = actions
  const blogPostTemplate = path.resolve(`./src/templates/template-blog-post.js`)
  const projectEntryTemplate = path.resolve(
    `./src/templates/template-project-entry.js`
  )
  const result = await graphql(`
    query {
      posts: allMdx(
        sort: { order: DESC, fields: [frontmatter___date, fields___slug] }
        limit: 10000
        filter: { fileAbsolutePath: { regex: "/content/posts/" } }
      ) {
        edges {
          node {
            id
            fields {
              slug
            }
            frontmatter {
              title
            }
          }
        }
      }
      projects: allMdx(
        sort: { order: DESC, fields: [frontmatter___date, fields___slug] }
        limit: 1000
        filter: { fileAbsolutePath: { regex: "/content/projects/" } }
      ) {
        edges {
          node {
            id
            fields {
              slug
            }
            frontmatter {
              title
            }
          }
        }
      }
    }
  `)
  if (result.errors) {
    reporter.panicOnBuild(
      '  ERROR: Loading "createPages" query.'
    )
  }
  const posts = result.data.posts.edges
  const projects = result.data.projects.edges
  posts.forEach(({ node }) => {
    createPage({
      path: node.fields.slug,
      component: blogPostTemplate,
      context: { id: node.id },
    })
  })

  projects.forEach(({ node }) => {
    createPage({
      path: node.fields.slug,
      component: projectEntryTemplate,
      context: { id: node.id },
    })
  })
}

My (redacted) gatsby-config.js file:

module.exports = {
  plugins: [
    {
      resolve: `gatsby-source-filesystem`,
      options: {
        name: `posts`,
        path: `${__dirname}/content/posts/`,
      },
    },
    {
      resolve: "gatsby-source-filesystem",
      options: {
        name: `projects`,
        path: `${__dirname}/content/projects/`,
      },
    },
  ],
}

Although this works fine and I'm not getting any errors, I'm not sure using two if-statements in gatsby-node.js is the right approach to generating pages for multiple content types. I've searched online and I have not yet found a guide or 'best-practices' when it comes to handling multiple content types with GatsbyJS. Please share if you have!

Could you guys help me out? Am I on the right track or is there improvement to be found?

Cheers!

There's definitely a lot of duplicate code in those 2 ifs. I'd extract it into function and pass different things as parameters.

Then code would look let's say:

//not sure about name of function and parameters
function nodeFieldPerType(filePath, getNodePath) {
  if (
    node.internal.type === "Mdx" &&
    node.fileAbsolutePath.includes(filePath)
  ) {
    const value = createFilePath({ node, getNode })
    createNodeField({
      name: "slug",
      node,
      value: getNodePath(value),
    })
  }
}

Then you call it like like:

nodeFieldPerType(`content/posts`, value => `/blog${value}`);
nodeFieldPerType(`content/projects`, value => `/project${value}`);

This cleans it up a bit. If you keep adding more possible branches, you may need to add more parameters. You can go further to make this as a static configuration and put this into configuration structure, that you fill in advance. Then in your code you just pick the one you need and call it.
Ex:

config = {
`content/posts`: { getNodePath:  value => `/blog${value}` }, //object on purpose so that you can add more parameters later
`content/projects`: { getNodePath: value => `/project${value}` },
}

And then instead of your ifs you check if filePath exists in this config and if it does, you call function with parameters in config.


Implementation of mergesort in Java I'd like to know if the following code is a good implementation of MergeSort? I tried some examples and the code was right, so I guess that the algorithm works correctly.

public static int[] myMerge (int[] array, int[] array2){

    int[] giveback = new int[array.length + array2.length];
    int i = 0;
    int j = 0;

    for (int x = 0; x < giveback.length; x++){

        if (array[i] >= array2[j]){

            giveback[x] = array2[j];
            j++;

        }
        else{

            giveback[x] = array[i];
            i++;
        }

        if (i == array.length){

            x++;
            for(int c = j; c < array2.length; c++){

                giveback[x] = array2[c];
                x++;
            }

            return giveback;
        }

        if (j == array2.length){

            x++;
            for (int b = i; b < array.length; b++){

                giveback[x] = array[b];
                x++;
            }
            return giveback;
        }

    }
    return giveback;
}

public static int[] myMergeSort (int[] array){

    if (array.length <= 1 ){

        return array;
    }

    if (array.length % 2 == 0){

        int[] right = new int[array.length/2];
        int[] left = new int[array.length/2];
        int counter = 0;

        for (int i = 0; i < array.length/2; i++){

            left[i] = array[i];

        }

        for (int j = array.length/2; j < array.length; j++){

            right[counter] = array[j];
            counter++;

        }
        return myMerge(myMergeSort(right),myMergeSort(left));
    }
    else{

        int[] right = new int[array.length/2];
        int[] left = new int[array.length/2];
        int counter2 = 0;

        for(int i = 0; i < array.length/2 +1; i++){

            left[i] = array[i];
        }

        for(int j = array.length/2 +1; j < array.length; j++){

            right[counter2]=array[j];
            counter2++;
        }
        return myMerge(myMergeSort(right),myMergeSort(left));
    }
}

You don't have to distinguish if array.length if even or odd.
Just do:

    int[] right = new int[array.length/2];
    int[] left = new int[array.length - array.length/2];

It will make the two tables to have exactly array.length items together, divided by half as precisely as possible.


SHA-1 and MD5 BruteForce in Python 3.7 I've made a python program that can brute-force both MD5 and SHA1 hashes, and I would like know if I could make the program faster or any way I could improve it.
I'm still a beginner in python and in coding in general
(this program is only for learning purposes).

The code:

import time
import string
import hashlib
ready = False
start = time.time()
chars = list(string.printable)[:95]
base = len(chars)
n = 0
hashmethod = 0
password = ""
solved = False
quit = ""
while ready != True:
    password = input("Enter a valid MD5 or SHA-1 hash:")

if len(password) == 32:
    ready = True
elif len(password) == 40:
    ready = True
    hashmethod = 2
else:
    continue

def numberToBase(n, b):  # converts number N base 10 to a list of digits base b
    digits = []
    while n:
        digits.append(int(n % b))
        n //= b
    return digits[::-1]

# check edge cases like empty, or 0
if password == '':
    print('Your password is empty')
    solved = True

# begin systematically checking passwords
while not solved:
    lst = numberToBase(n, base)
    word = ''
    for x in lst:
        word += str(chars[x])
    if hashmethod == 2:
        hashedGuess = hashlib.sha1(bytes(word, 'utf-8')).hexdigest()

    else:
        hashedGuess = hashlib.md5(bytes(word, 'utf-8')).hexdigest()
        print(word)
    if password == hashedGuess:
        solved = True
        print('-Stats-')
        print('Pass: ' + word)
        print('Attempts: ' + str(n))
        print('time: ' + str((time.time() - start)) + ' sec')
        while quit != " QUIT":
            quit = input('Type <QUIT> to quit')
    else:
        n += 1

For hash brute-forcing, the short answer is "don't do it in Python". You'll get massive performance gains in switching to C or C++.

Since this is for learning purposes, reviewing your Python:

For legibility, add a newline between your imports and your code.
Consider putting your global-scope code into a main() function plus subdividing some of the code into other functions.
Does your code actually run? You have a loop on ready, but you don't modify its value, so I think it's going to hang there. I suspect that you've broken your loop's indentation, and that some statements below belong in the loop.
Instead of setting ready to later break out of the loop, you can simply break. You can get rid of your else/continue.


Method to remove comments from a string line There is some language where the following are considered comments:

[str] // comment
[str] /* comment */ [str]
[str] # comment
[str] /* comment1
comment2 */ [str]

where [str] is 0 or more occurrences of some string.

I'm reading a file line by line, and need to remove the comments from the whole file (the file is large so I cannot keep the whole content in memory).

I have written a method that receives each line, removes the comments from it, and returns the remaining string.

(Here s_bInComment is a static variable in Parser class with initial value of false)

void Parser::removeComments(std::string &str)
{
    // see if we're still in comment lines, i.e. not reached */
    auto commentEnd = str.find("*/");
    if (s_bInComment && commentEnd == std::string::npos)
    {
        str.clear();
        return;
    }

    std::string result;

    const auto iStrLen = str.length();
    for (std::size_t idx = 0; idx < iStrLen; ++idx)
    {
        if (str[idx] == '/')
        {
            if (idx + 1 < iStrLen)
            {
                if (str[idx + 1] == '/')
                {
                    // we found //
                    // take the part of the string before the comment, if any
                    str = str.substr(0, idx);
                    break;
                }

                if (str[idx + 1] == '*')
                {
                    // see if */ is in the same line
                    commentEnd = str.find("*/");
                    if (commentEnd == std::string::npos)
                    {
                        // multiline comment
                        s_bInComment = true;
                        str = str.substr(0, idx);
                    }
                    else
                    {
                        // remove the comment
                        str = result + str.substr(commentEnd + 2);

                        // recurse on the remaining string
                        removeComments(str);
                    }

                    break;
                }
            }
        }
        else if (str[idx] == '*')
        {
            if (idx + 1 < iStrLen)
            {
                if (str[idx + 1] == '/')
                {
                    // we have found */, comment ended
                    s_bInComment = false;

                    // take the part of the string after the comment, if any
                    str = str.substr(idx + 2);

                    // recurse on the remaining string
                    removeComments(str);

                    break;
                }
            }
        }
        else if (str[idx] == '#')
        {
            // if there's a text before #, it should be accumulated in result
            str = result.size() ? result : std::string();
            break;
        }
        else if (!s_bInComment)
            result += str[idx];
    }
}

I have tested my code with the following input lines:

a /* b */ c
a /* b */
a // b
a /* b
c
*/
// a
a /* b
*/ c
# a
a # b
a /* a */ b /* b */ c

Please let me know if there's room for improvement in one or other part.

I believe it's a good example of a problem that you can represent with a state-machine. What you do with the next character depends on the state you're in:

if you're outside of a comment, then you copy the character unless it's a / or a #
if the last character was a / outside a comment, then test if the new character is a * or a /
if you're inside a single line comment, you ignore it unless it's a newline
if you're inside a multi-line comment, you ignore it unless it's a star
if you've just read a * inside a multi-line comment then you need to check if the new char isn't a /

State-machines aren't always the most elegant way of coding, but they're robust, easy to read and blazingly fast. So how would you implement it in this case?

First, don't read the input line by line. It is probably a mistake since crucial information resides in the previous line, if there's one: you can't know, looking at a line, if a multi-line comment was opened in the previous one. Also, it will force you to perform memory management each time you read a line (you need to check if the output string is big enough and allocate more memory if it isn't), which will slow down your program, whereas looking character by character will allow you greater flexibility (any object exposing iterators on characters is acceptable) and prevent allocations if they aren't needed.

That brings us to the signature:

template <typename InputIt, typename OutputIt>
OutputIt copy_without_comments(InputIt first, InputIt last, OutputIt out)

The OutputIt can be many things: an iterator on an allocated buffer, an iterator constructing a string, an iterator on a stream...

The state machine itself is generally implemented with an enum specifying the possible states:

enum class State : char { SlashOutsideComment, StarInsideComment, SingleLineComment, MultiLineComment, NotAComment };

Then it's only a big switch to know how to handle the next character:

 switch (state) {
            case State::SlashOutsideComment:
                if (*first == '/') state = State::SingleLineComment;
                else if (*first == '*') state = State::MultiLineComment;
                else {
                    state = State::NotAComment;
                    *out++ = '/';
                    *out++ = *first;
                }
                break;
            case State::StarInsideComment:
                if (*first == '/') state = State::NotAComment;
                else state = State::MultiLineComment;
                break;
            case State::NotAComment:
                if (*first == '#') state = State::SingleLineComment;
                else if (*first == '/') state = State::SlashOutsidedComment;
                else *out++ = *first;
                break;
            case State::SingleLineComment:
                if (*first == '\n') {
                    state = State::NotAComment;
                    *out++ = '\n';
                }
                break;
            case State::MultiLineComment:
                if (*first == '*') state = State::StarInsideComment;
        }

Link to test and play with: https://wandbox.org/permlink/iXC7DWaU8Tk8jrf3


Load from dictionary or database if not found Today I have coded a simple function that will get a room model, if its already been loaded in to the dictionary then it will just return it, if its not in the dictionary if will try its best to load it from the database. I just wondered is this the best way I can do ths? or is there a better way...

public bool TryGetModel(string modelName, out RoomModel model)
{
    if (_roomModels.ContainsKey(modelName))
    {
        _roomModels.TryGetValue(modelName, out model);
        return true;
    }

    using (var mysqlConnection = Sahara.GetServer().GetMySql().GetConnection())
    {
        mysqlConnection.SetQuery("SELECT id,door_x,door_y,door_z,door_dir,heightmap,public_items,club_only,poolmap,`wall_height` FROM `room_models` WHERE `custom`");
        var modelRow = mysqlConnection.GetRow();

        if (modelRow == null)
        {
            model = null;
            return false;
        }

        model = new RoomModel(Convert.ToInt32(modelRow["door_x"]), Convert.ToInt32(modelRow["door_y"]),
            Convert.ToDouble(modelRow["door_z"]), Convert.ToInt32(modelRow["door_dir"]),
            Convert.ToString(modelRow["heightmap"]), Convert.ToString(modelRow["public_items"]),
            Convert.ToInt32(modelRow["club_only"]).ToString() == "1", Convert.ToString(modelRow["poolmap"]),
            Convert.ToInt32(modelRow["wall_height"]));

        _roomModels.Add(Convert.ToString(modelRow["id"]), model); // save it for next time!!
        return true;
    }
}

Using ContainsKey() together with TryGetValue() seems a little bit too much. Just change it like so

if (_roomModels.TryGetValue(modelName, out model))
{
    return true;
}

which is much easier to read and faster for items which are in the dictionary.

If the columns in the database allow null values you should check first with Convert.IsDBNull() so the method doesn't blow in the users face at any of the Convert.ToXX() methods.

The Where clause of the sql query looks like it is not working.
You should at least let the columns have some space to breathe which makes the query easier to read.


Calculate sum of price on distant relation I have a booking with a method total which calculates the total price of the booking from the sum of all the prices of activities that belong to appointments. A booking has many appointments and the following seems to give the right answer, but it seems horrid.  How can I improve this?

  def total
    # TODO  shouldn't this be a db query?
    total = 0
    self.appointments.each do |appointment|
        unless appointment.activity.price.nil?
            total += appointment.activity.price.to_f
        end
    end
    total
  end

You can do this:

def total
  self.appointments.joins(:activity).sum(:price)
end

You could perhaps simplify it a little, by making an explicit 2nd order has_many on your booking model:

has_many :appointments
has_many :activities, :through => :appointments

Then you can just do:

def total
  self.activities.sum(:price)
end

In either case, it'll end up as a single DB-query.


PHP: checking for $_GET variables with if statements This code gets me where I need to be, but my word it is ugly.

I am checking for the existence of two $_GET variables with if() statements. I am setting the variables to empty strings first so I do not get an error when I echo them back.

$getvar1 ='';
$getvar2 ='';
$np ='';

if(isset($_GET['getvar1'])) $page_id = $_GET['getvar2'];
if(isset($_GET['getvar2'])) $route = $_GET['getvar2'];

if(($getvar1 == '' && $getvar1 == '') || $getvar1 == '4')  $np = 'np';

    echo "$getvar1 $getvar2 $np";

Is there a better way to declare the variables than setting them to empty strings?

Is there a better way to check and set the $_GET variables?

If it's a small app something like that would probably be better:

$getvar = isset($_GET['getvar']) ? $_GET['getvar'] : 'somedefault';

If you want to test more stuff:

$getvar = isset($_GET['getvar']) && $JUST_PUT_IT_HERE ? $_GET['getvar'] : 'somedefault';


Offset vs Cursor Pagination I am trying to compare any performance gain(s) from moving a current offset-based pagination endpoint to a cursor based one.

I have the following ruby code:

@limit = 25
@max_pages = 200

def query
  @q ||= MyDataBaseClass.where(status: "open").order(:id)
end

def offset_paginatiion
  # count is currently an implementation for this appraoch
  total = query.count
  ctr = 0
  page_size = 1
  while page_size < @max_pages
    query.limit(@limit).offset(ctr).load
    ctr += @limit
    page_size += 1
  end
end

def keyset_pagination
  current_page = query.limit(@limit)

  page_size = 1
  while true
    break if page_size >= @max_pages
    page_size += 1
    last_id = current_page.last.id
    current_page = query.where("id > ?", last_id).limit(@limit).load
  end
end

require 'benchmark/ips'

Benchmark.ips do |x|
  x.report("offset_pagination") { offset_paginatiion }
  x.report("keyset_pagination") { keyset_pagination }
  x.compare!
end

I've seeded the database with 1 million rows but you can see here I'm only trying to page up to 200 pages (or 5,000 results).

The two methods are returning the exact same data in the same order so they are functionally equivalent and after running the benchmark are performing nearly the same:

│Calculating -------------------------------------
│   offset_pagination      0.009  (± 0.0%) i/s -      1.000  in 110.100327s
│   keyset_pagination      0.009  (± 0.0%) i/s -      1.000  in 112.732942s
│
│Comparison:
│   offset_pagination:        0.0 i/s
│   keyset_pagination:        0.0 i/s - 1.02x  slower

I was under the impression performance (from numerous articles) was a benefit to moving to cursor pagination but have yet to been able to reproduce - what am I doing worng?

I'm not surprised at your results, bar a few minor details that any decent database will optimize away your implementations are the same. You are also basically just re-implementing the find_each method.

Note, you might find a real difference if you used database cursors but that is a completely different thing.

The real difference between the two is that cursor based pagination responds better when the underlying data changes. i.e. using offset based pagination you might have a record included twice or skipped if a record is inserted or deleted by another process.


Start a process in Elisp How can the following code be made idiomatic in ELisp? Can the temporary f variable be removed?

(global-set-key [f3]
  (lambda () (interactive)
    (let ((f (ffap-file-at-point)))
      (start-process "test" "output" "mpv" f))))

You can remove the let, as you only reference f once.

I'd also make it a named function; this makes it easier for you to update the definition, or to bind it to additional keys:

(defun start-mpv--file-at-point ()
  (interactive)
  (start-process "test" "output" "mpv" (ffap-file-at-point)))

(global-set-key [f3] 'start-mpv--file-at-point)

You might also want to look at the implementation of shell-command for inspiration.  Some things it does that might interest you:

killing any process that's already writing to the output buffer
erasing the current contents of that buffer
setting the comint output filter to handle carriage motion
making the output buffer visible


Balanced Parentheses checker in Python I'd appreciate feedback on the code itself (style, performance optimization (both space and time)), and also on the algorithm (because I think this is typically implemented using a stack).

The assumption I'm making here is that there are only two constraints to having balanced parentheses; one is that there be the same number of opening and closing parentheses, the other one that there not be at any point more closing parentheses than opening ones.

With this in mind, I implement my balanced parentheses checker using a simple counter, which is incremented with each opening paren, and decremented with each closing paren.

The two "checks" within the function are that the counter never go negative (return False if it does at any point), and that at the end the counter be 0.

def paren_checker(string):
    counter = 0
    for c in string:
        if c == '(':
            counter += 1
        elif c == ')':
            counter -= 1
        if counter < 0:
            return False

    if counter == 0:
        return True
    return False

I would replace your last lines with return counter == 0 as it is more compact and easier to read


Getting path array for given value in nested Object I'm wondering how I can improve my getPath function (I found it on SO and adjusted it to my needs - biggest change was returning an array, used to be a string).

It takes an object and a value as inputs and returns an array of object keys for the first matching value they find, otherwise false.

My focus isn't primarily the speed.
Focus is: As short but at the same time as easy to understand as possible without having to write many comments.

I'm using JS before ECMAScript 6

var nestedObj = {
  "rrr": {
    "ddd": {
      "aa    bbbb": {
        1: 30009463
      },
      "cc    dddd.": {
        1: 30010338
      },
      "e     fff": {
        1: 30007744
      },
      "d-Pool": {
        1: 30018363,
        2: 30017133,
        3: 30013107
      },
      "e   e g": {
        1: 30011707,
        2: 30017137
      },
      "f   f.-f. (f)": {
        1: 30012329
      },
      "g": {
        1: 30011894
      }
    }
  }
};

var result = getPath(nestedObj, 30017137);
document.getElementById("result").innerHTML = result;

function getPath(obj, value, path) {
  if (typeof obj !== 'object') {
    return false;
  }
  for (var key in obj) {
    if (obj.hasOwnProperty(key)) {
      var t = path;
      var v = obj[key];
      //SLICE is super important, otherwise newPath will reference path
      var newPath = path ? path.slice() : [];
      newPath.push(key);
      if (v === value) {
        return newPath;
      } else if (typeof v !== 'object') {
        newPath = t;
      }
      var res = getPath(v, value, newPath);
      if (res) {
        return res;
      }
    }
  }
  return false;
}

I wrote that one comment as a reminder to myself because I spent too much time wondering why the function didn't work before figuring this out.

Some thoughts:

This is not truly a recursive function, in that you are not passing the path value when first calling the function.  This may hint that you should really just recurse privately within the function itself such that you can better control "state" (mainly that of path and whether or not match has been found) and simplify things.
You can design away pretty much all of the temporary variables you use within the loop.
Why are you making a check for hasOwnProperty?  It seems like perhaps you are trying to exclude Arrays, Numbers, etc.  here? If that is the case, just check for the object's constructor rather than typeof up front and get rid of this.
Consider throwing on case where non-object is encountered in input.
Why mix return types?  If you are expecting an array as a result, why not have empty array represent "no result" case?

Putting it all together might yield something like:

function getPath(obj, value) {
    if(obj.constructor !== Object) {
        throw new TypeError('getPath() can only operate on object with Object as constructor');
    }
    var path = [];
    var found = false;

    function search(haystack) {
        for (var key in haystack) {
            path.push(key);
            if(haystack[key] === value) {
                found = true;
                break;
            }
            if(haystack[key].constructor === Object) {
                search(haystack[key]);
                if(found) break;
            }
            path.pop();
        }
    }

    search(obj);
    return path;
    /*
    Or alternately if you want to keep mixed return
    return found ? path : false;
    */
}

var nestedObj = {
  "rrr": {
    "ddd": {
      "aa    bbbb": {
        1: 30009463
      },
      "cc    dddd.": {
        1: 30010338
      },
      "e     fff": {
        1: 30007744
      },
      "d-Pool": {
        1: 30018363,
        2: 30017133,
        3: 30013107
      },
      "e   e g": {
        1: 30011707,
        2: 30017137
      },
      "f   f.-f. (f)": {
        1: 30012329
      },
      "g": {
        1: 30011894
      }
    }
  }
};

console.log(getPath(nestedObj, 30017137));
console.log(getPath(nestedObj, 'no value'));