String split for street addresses I have 856k addresses of pattern:

660 1st Ave   New York, NY 10016

(with 3 spaces between Ave and New York)

I need to parse and store parts as:

Street, City, State, Zip

I tried to split through regex but it was super slow, so I went traditional:

def splitAddress(address):
    split = '   '
    ad = address.strip()

    try:
       if(split in ad):
           addr = ad.split(split)
           if(len(addr)==2):
                addr1 = addr[1].strip().split(",")
                if len(addr1)==2:
                      addr2 = addr1[1].strip().split(" ")

                      if len(addr2)==2:
                           return addr[0], addr1[0], addr2[0], addr2[1]
                      else:
                           return "0","0","0","0"
                else:
                     return "0","0","0","0"
           else:
               return "0","0","0","0"
       else:
           return "0","0","0","0"
    except ValueError as e:
          print(addr[0], addr1[0], addr2[0], addr2[1])
          print(e)

I then got the parts as:

df['STREET'],df['CITY'], df['STATE'], df['ZIP'] = zip(*df['ADDRESS'].map(splitAddress))

But this is turning out to be pretty slow (it takes 18 mins for 856k addresses).

How did you try implementing your regex?

You can take advantage of panda's str methods to vectorize your regex instead of mapping the function over each element of the dataframe.

df = pd.DataFrame(index=np.arange(900000))
df["address"] = "660 1st Ave   New York, NY 10016"

With a dataframe with 900000 addresses,

df.address.str.extract("regex_pattern", expand=True)

will extract your regex from each row of the 'address' column in your dataframe.
Something like,

df[["street", "city", "state", "zip"]] = df.address.str.extract('(.+)[ ]{3}(.+)\,[ ]([a-zA-z]{2})[ ]([0-9]{5})',expand=True)

runs over the entire dataframe in:

  %timeit
  7.2 s ± 81.4 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

Basically, you should look for any opportunity to take advantage of panda's vectorized optimizations (string operations, datetime operations, masks, etc.) as described in the docs: https://pandas.pydata.org/pandas-docs/stable/basics.html#vectorized-string-methods

If at any point you are trying to apply a function to each element in the dataframe/series, stop and check to see if there is already a built-in method that implements what you want.


Sort three numbers using only if-statements Beginner here, trying to make a small program that sorts three numbers from smallest to largest only by using ifs. Any thoughts on how to improve this?

#include <iostream>

using namespace std;
int main ()
{
    int num1, num2, num3;
    int smallest, middle, biggest;
    cin >> num1 >> num2 >> num3;

    cout << endl;

    if ((num1 < num2) && (num1 < num3))
    {
        smallest = num1;
        if (num2 > num3)
        {
            biggest = num2;
            middle = num3;
        }
    }
    if ((num1 < num2) && (num3 << num1))
    {
        smallest = num1;
        if (num2 < num3)
        {
            middle = num2;
            biggest = num3;
        }
    }
    if ((num1 > num2) && (num3 > num1))
    {
        middle = num1;
        if (num2 < num3)
        {
            smallest = num2;
            biggest = num3;
        }
    }
    if ((num1 < num2) && (num3 < num1))
    {
        middle = num1;
        if (num2 > num3)
        {
            biggest = num2;
            smallest = num3;
        }
    }
    if ((num1 > num2) && (num1 > num3))
    {
        biggest = num1;
        if (num3 > num2)
        {
            middle = num3;
            smallest = num2;
        }
    }

    if ((num1 > num2) && (num1 > num3))
    {
        biggest = num1;
        if (num2 > num3)
        {
            middle = num2;
            smallest = num3;
        }
    }

    cout << smallest << ", " << middle << ", " << biggest << endl;

    return 0;
}

I will try to take several things into account but to keep things simple. A few remarks:

First, it seems that you have a typo here:

if ((num1 < num2) && (num3 << num1))

I think that you meant num3 < num1 instead of num3 << num1 in your second condition.
As @Josay says, you better write small functions. I would add that you better separate the input/output operations (that you can keep in main) and the sorting function. In my opinion, this would be a good enough signature for the sorting function:

void sort3(int& a, int& b, int& c);

You would give three variables to it and it would sort them in-place so that you end up with \$ a \le b \le c \$.
Now, let's choose an algorithm. For three values, the easiest is to implement a bubble sort which shouldn't be really slower than other algorithms (when you want to sort more values, it becomes horribly slower though).

void sort3(int& a, int& b, int& c)
{
    if (a > b)
    {
        std::swap(a, b);
    }
    if (b > c)
    {
        std::swap(b, c);
    }
    if (a > b)
    {
        std::swap(a, b);
    }
}

Most sorting algorithms heavily rely on swapping values. The one I just implemented sorts your values with only three comparisons and at most three swaps and is, in my opinion, far simpler to understand than what you had.

Note that there are more efficient algorithms but I deliberately chose to present one that is not that bad while easy to understand.
This is not a problem in your case, but we can't stress out enough that using namespace std; is often considered bad practice. It is the case when used in a header file, especially in a library header file since it will pollute the global namespace of every file including it. That's not a problem for you since you're probably doing everything in a .cpp file but it's better to keep that in mind.
You don't need to return 0; at the end of main. If the compiler reaches the end of the main function without having encountered a return statement, it automagically adds a return 0; for you. Note that it only works with main though. Dropping this line is an interesting way to document that your program cannot return error codes and that it will only ever return 0.


Checking whether relations are equal to foreign fields using LINQ Any() Can you find a more understandable way to write the following one-liner?

Enumerable.Range(0, relation.Fields.Count).
           Any(i => relation.Fields[i] != relation.ForeignFields[i])

but still preserving its brevity?

Simply put, I wanna cycle on relation.Fields collection to see if their string elements are one-by-one equal to relation.ForeignFields ones.

The reason I had to use Enumerable.Range is that I need an index, in order to cycle the two collections.

Note: they have for sure the same count.

I wrote an extension method based on @t3chb0t's answer

Can you not simply do SequenceEqual?

var list = new List<string> { "test1", "test2", "test3" };
var list2 = new List<string> { "test1", "test2", "test3" };

var areListsEqual = list.SequenceEqual(list2);


Angular component to render currency or date values I have an Angular component and it is just a label which renders a value and if it has an associated pipe, the output of the pipe should be rendered as value of the label. But I don't want to declare all the pipes in the component. Currently I would like to use the currency pipe and the date pipe with their parameters. But maybe I can extend the component to use other pipes.

Would you pass the pipes as input parameter of the component or what would be your approach to achieve that? Any idea?

app.component.ts

<app-custom-label value="10000" title="My Title for a Currency render" datatype="currency" param="EUR"></app-custom-label>

<app-custom-label value="01.10.1980" title="My Title for a Date render" datatype="date" param="dd MMMM"></app-custom-label>

custom.label.component.html

<h1>{{ title }}</h1>
<label> {{ value }} </label>

custom.label.component.ts

import { Component, Input, OnInit  } from '@angular/core';
import { DataType } from './type';
import { CurrencyPipe, DatePipe } from '@angular/common';
import { registerLocaleData } from '@angular/common';
import localeDe from '@angular/common/locales/de';
import localeDeExtra from '@angular/common/locales/extra/de';

registerLocaleData(localeDe, 'de-DE', localeDeExtra);

@Component({
  selector: 'app-custom-label',
  templateUrl: './custom.label.component.html'
})
export class CustomLabelComponent implements OnInit {
  @Input() value: string;
  @Input() title: string;
  @Input() datatype: DataType;
  @Input() param: string;

  constructor() {}

  ngOnInit(){
    if (this.datatype === "currency") {
      this.value = (new CurrencyPipe('de-DE')).transform(this.value, this.param, true);
    } else if (this.datatype === "date") {
      this.value = (new DatePipe('de-DE')).transform(this.value, this.param);
    }
  }
}

Here you have my StackBlitz. If you can see, I pass a type of a pipe and its params as parameters, if I want to use any pipe. The component could also render a label without using a pipe.

https://stackblitz.com/edit/angular-label-render

The syntax of both the pipes:

Currency Pipe

{{ value_expression | currency [ : currencyCode [ : display [ : digitsInfo [ : locale ] ] ] ] }}

and Date Pipe

{{ value_expression | date [ : format [ : timezone [ : locale ] ] ] }}

In my understanding, we must use pipes to integrate directly into the "templates" and avoid writing excess code in the component.
In your approach, passing the pipe names and the other parameters appears to me as a redundant step.

My approach would be:

<app-custom-label value="{{10000 | currency:'EUR':true:null:'de-DE'}}" title="My Title for a Currency render"></app-custom-label>
<app-custom-label value="{{'01.10.1980' | date:'dd MMMM':null:'de-DE'}}" title="My Title for a Date render">


A better way to convert an SQLObject instance into a dict I've got this function which returns an SQLObject instance as a dict (taking into account inherited classes, properties, etc).  It doesn't work right now on SQLObject classes which inherit from another class but add new attributes, so that's a work in progress, but I feel like this could be done smarter/better in Python.

What do you think?

PS the purpose of this is to allow a generic presentation of an object as a JSON document.  It returns a dict which can be then given to json.dumps.  If you've got a different way to convert an SQLObject to a JSON doc, that would be cool as well

def sqlobject_to_dict(obj):
    obj_dict = {}
    cls_name = type(obj)
    has_props = False
    for attr in vars(cls_name):
        attr_parent = type(getattr(obj, attr)).__bases__[0]
        if isinstance(getattr(cls_name, attr), property):
            has_props = True
            if isinstance(getattr(obj, attr), Decimal):
                obj_dict[attr] = float(getattr(obj, attr))
            elif isinstance(getattr(obj, attr), list):
                dict_list = []
                for list_item in getattr(obj, attr):
                    dict_list.append(sqlobject_to_dict(list_item))
                obj_dict[attr] = dict_list
            elif isinstance(getattr(obj, attr), dict):
                dict_dict = {}
                for key, val in getattr(obj, attr):
                    dict_dict[key] = sqlobject_to_dict(val)
                obj_dict[attr] = dict_dict
            elif attr_parent == SQLObject:
                obj_dict[attr] = sqlobject_to_dict(getattr(obj, attr))
            else:
                obj_dict[attr] = getattr(obj, attr)
        elif not has_props and attr_parent != SQLObject:
            if "_get_" in attr:
                attr_name = '_'.join(attr.split('_', 3)[3:])
                if hasattr(obj, attr_name):
                    obj_dict[attr_name] = getattr(obj, attr_name)
    return obj_dict

def sqlobject_to_dict(obj):
    obj_dict = {}
    cls_name = type(obj)

That's not a name, its a class object

    has_props = False
    for attr in vars(cls_name):
        attr_parent = type(getattr(obj, attr)).__bases__[0]

You have getattr(obj, attr) over and over and over. Do it once, and store it in a local variable

        if isinstance(getattr(cls_name, attr), property):
            has_props = True
            if isinstance(getattr(obj, attr), Decimal):
                obj_dict[attr] = float(getattr(obj, attr))
            elif isinstance(getattr(obj, attr), list):
                dict_list = []
                for list_item in getattr(obj, attr):
                    dict_list.append(sqlobject_to_dict(list_item))
                obj_dict[attr] = dict_list

I recommend doing obj_dict[attr] = map(sql_object_dict, getattr(obj, attr))

            elif isinstance(getattr(obj, attr), dict):
                dict_dict = {}
                for key, val in getattr(obj, attr):

Don't you need .items()?
                        dict_dict[key] = sqlobject_to_dict(val)
                    obj_dict[attr] = dict_dict

I'd do obj_dict[attr] = dict( (key, sql_object_to_dict(value)) for key, value in getattr(obj, attr) )
                elif attr_parent == SQLObject:

Why did you fetch attr_parent instead of using isinstance?

                obj_dict[attr] = sqlobject_to_dict(getattr(obj, attr))
            else:
                obj_dict[attr] = getattr(obj, attr)

Is a default really a good idea? There may some sort of attribute with isn't valid JSON that makes it through.

        elif not has_props and attr_parent != SQLObject:

            if "_get_" in attr:
                attr_name = '_'.join(attr.split('_', 3)[3:])

Thats a confusing way to do that. I'm sure what exactly the form you are trying to match here is,  but I think this is a use case for regular expressions.

                if hasattr(obj, attr_name):
                    obj_dict[attr_name] = getattr(obj, attr_name)

So I'm guessing you've got various _get_foo() functions which are invoked by some sort of __getattr__ magic. You stop checking for this once you find a property. (Probably newer objects use properties instead.) I would consider not doing the has_props thing. It makes your code less clear, and probably doesn't make much difference performance wise.

    return obj_dict


Determine if the student has passing grades I made a program in Java that asks for the student's name, course, and grades. After the required text inputs are filled, the program will then show the result. If the student passed, the program will show "PASSED"; if the student is failing, the program will show "ACADEMIC WARNING"; if the student failed, the program will show "FAILED".
So this is the code:
/*
 * Java Grades by fosionef
 */

import javax.swing.JOptionPane;
import java.util.Scanner;

public class JavaGrades {
    public static void main(String[] args) {
        getStudentName();
        getStudentCourse();
        getGrades();
    }

    // get student name
    public static void getStudentName() {
        Scanner input = new Scanner(System.in);
        String name = JOptionPane.showInputDialog(null,"Input Name: ");
        //String name = input.nextLine();
        JOptionPane.showMessageDialog(null,"Name: " + name.toUpperCase());
    }

    // get student course
    public static void getStudentCourse() {
        Scanner input = new Scanner(System.in);
        String course = JOptionPane.showInputDialog(null,"Input Course: ");
        //String course = input.nextLine();
        JOptionPane.showMessageDialog(null,"Course: " + course.toUpperCase());
    }

    // get student grades
    public static void getGrades() {
        Scanner input = new Scanner(System.in);
        double grades = Double.parseDouble(JOptionPane.showInputDialog(null,"Input grades: "));
        //double grades = input.nextDouble();

        if(grades == 90 || grades >= 80) {
            JOptionPane.showMessageDialog(null,grades + ": PASSED!");
        }else if(grades == 79 || grades >= 75) {
            JOptionPane.showMessageDialog(null, grades + ": ACADEMIC WARNING!");
        }else if(grades <= 74) {
            JOptionPane.showMessageDialog(null,grades + ": FAILED!");
        }else{
            JOptionPane.showMessageDialog(null, "Please input the required");
        }
    }
}

How was the code? I am looking forward to corrections and suggestions. That way I can improve my coding skills. Thank you.

Reduce code repetition / redundant checks
Your if statements are redundant. If grade = 90, it already satisfies the grade >= 80, so just use that. No need to check if it equals 90 exactly because that's already covered.
Instead of having four different lines where you show a message dialog, have a string that you assign the message to and print that at the end? Something like this:
String message;
if (grades >= 80) {
    message = grades + ": Passed!";
} else if (grades >= 75) {
    message = grades + ": Academic Warning!";
} else {
    message = grades + ": Failed!";
}
JOptionPage.showMessageDialog(null, message);

Reduce unused code
You created a Scanner in all of your functions, but never use them. I'd remove them for code clarity.


Determine which sentences contain the set of phrases I am trying to solve this HackerRank challenge which asks to write a function that prints the indexes of all sentences which contain the query words. For example, if the inputs to the function are:

textQueries(sentences, queries)
textQueries(['jim likes mary','kate likes tom', 'tom does not like jim'],['jim tom', 'likes'])

Then the function will print

2
0 1

If there is no sentence contains the query words, -1 will be printed

The constraints for the inputs are:

The length of the sentences and queries lists will not exceed 10000.
The number of words in any sentence or query is from 1 to 10 words.
Each word has at most 11 characters.
No word appears in more than 10 sentences.
Each word consists of uppercase and lowercase English letters only.

My attempt so far is to create a hashmap which maps each single query word with a set that contains the indexes to the sentences which contain that word, then I intersect those sets with each other to get the final set. My code runs fine, but it is slow for large test cases. Is there any way I can have it optimized?

def textQueries(sentences, queries):
    word_map = {}
    for query in queries:
        query_words = query.split()
        final_set = set([])
        set_modified = False
        for query_word in query_words:
            if query_word in word_map:
                if len(final_set) == 0 and not set_modified:
                    final_set = word_map[query_word]
                    set_modified = True
                    continue
                elif len(final_set) == 0 and set_modified:
                    break
                else:
                    final_set = final_set.intersection(word_map[query_word])
                    continue
            else:
                sentences_contain_word = []
                for index, sentence in enumerate(sentences):
                    words = sentence.split()
                    if query_word in words:
                        sentences_contain_word.append(index)
                        if len(sentences_contain_word) >= 10:
                            break
                word_map[query_word] = set(sentences_contain_word)
                if len(final_set) == 0 and not set_modified:
                    final_set = word_map[query_word]
                    set_modified = True
                elif len(final_set) == 0 and set_modified:
                    break
                else:
                    final_set = final_set.intersection(word_map[query_word])
        if len(final_set) == 0:
            print(-1)
        else:
            for index in sorted(final_set):
                print(str(index), end=' ')
            print()

Your code is fairly readable, but I find it quite dense. Here are some general things you could do to improve it:

Use vertical white space to separate different areas of the code.
Provide a docblock at the top outlining the algorithm you are using.
Use more than one function to break your code into smaller chunks.

With that advice out of the way, let's look at your performance with an eye to the worst possible case:

def textQueries(sentences, queries):
    for query in queries:             # Worst case: 10,000 times
        ...
        for query_word in query_words:  # Worst case: 10 times
            if query_word in word_map:
                    final_set.intersection  # Worst case: O(10,000) in C
            else:
                for ... in enumerate(sentences):  # Worst case: 10,000 times
                if ... else:
                    final_set.intersection  # Worst case: O(10,000) in C

According to me, your code runs worst case in \$O(Q \times W \times S)\$ where \$Q\$ is the number of queries, \$W\$ is the number of words in the query, and \$S\$ is the number of sentences. (The number of words in the sentences is hidden behind a split, but it's in C so we'll let it go by.)

There's no question you need to split the sentences and split the queries into separate words, somehow. But you are nesting your loops when the "best" way to loop is to do them one after the other. If you can convert from this:

for q in Q:
    for s in Q:
        pass

to something like this:

for s in S:
    pass

for q in Q:
    pass

You will have converted from \$O(S \times Q)\$ to \$O(S + Q)\$ which is considerably faster.

Your current approach is to define a set of sentences that contain a particular word. You cache these sets, in case the same words appear again. You intersect the sets for each word in the query, producing a final set of all the sentences that contain all the query words.

This is pretty much guaranteed to fail based on the instructions you quoted:

  No word appears in more than 10 sentences.

In other words, the probability of a "cache hit" is 10/10,000, or 0.1%. Any time you spend maintaining the cache is wasted time.

Suppose you just go through the sentences one time, and build the big dictionary of sets:

sentence_words = collections.defaultdict(set)
for i, s in enumerate(sentences):
    for w in s.split():
        sentence_words[s].add(i)

Then you can go through the query words and do what you were doing before, without any caching:

for q in queries:
    matches = functools.reduce(operator.and_, (sentence_words[w] for w in q.split()))
    matches = [-1] if not matches else sorted(matches)

This will seem very similar to what you were doing, but the difference is that these two loops are run one after the other, providing something like \$O(100,000 + 100,000)\$ performance instead of \$O(100,000 \times 10,000)\$. It's that last operator that makes all the difference.


Finding matches in Excel files, adding to a Pandas Dataframe I have a business requirement where I have to read excel files and match each cell with a given keyword list and find the matches and write it into a dataframe.

I have written the code, but when the excel size is increasing (7 MB - 5 sheets, 30k * 9), the code is taking unusually long time (40 minutes), and I know that it should since I have so many for loops, but is there anyway to optimize it?

I have tried many different ways to optimize the same, but nothing worked out well.

import pandas as pd
import re
import datetime
import numpy as np

raw_details_list = []
# there are around 10 keys and 600 values
keyword_dic = {"it spend": ['IT OpEx', 'OpEx'], "Infra": ["Dell", "Oracle"]}

def pattern(keys):
    return re.compile('|'.join([r'(?<!\w)%s(?!\w)' % re.escape(keys)]), flags=re.I)

# this is reading each cell of the excel and doing a match whether with the keyword_dic
def sheet_handler(path, page_count, row):
    try:
        global keyword_dic
        if isinstance(row, str):
            for category, keywords in keyword_dic.items():
                for keys in keywords:
                    r = pattern(keys)
                    words = r.findall(row)
                    # storing the matches in a list of dictionary
                    for i in words:
                        temp_dic = {"File Path": path, "Total Number of Page(s)/Slide(s)": page_count, "Keyword": keys,
                                    "IT Category":category}
                        raw_details_list.append(temp_dic)
    except Exception as e:
        pass

# reading the excel - all sheets and creating a dataframe
def xlsx_handler(path):
    # reading the excel file
    sheets_dict = pd.read_excel(path, sheet_name=None, header=None)
    sheet_count = len(sheets_dict)
    for name, sheet in sheets_dict.items():
        # removing float type columns
        sheet = sheet.loc[:, sheet.dtypes != np.float64]
        # reading each row, we have keep track of each cell's row and column details
        for index, row in sheet.iterrows():
            for i in range(len(row)):
                try:
                    # if the content is float, we can skip it
                    float(row[i])
                except Exception as e:
                    sheet_handler(path, sheet_count, row[i])

print(datetime.datetime.now())
xlsx_handler(r"D:\Backend Python\test\test.xlsx")
df = pd.DataFrame(raw_details_list)
print(datetime.datetime.now())

Sample data:

1   Oracle  ERP would be good   United states
2   Dell    Laptops UK
3   Vaio    Off-limit   India
4   Oracle 1.2  E-series    Chine

The output for this should somewhat look like this:

File Path   Pages   Keyword IT Category
D://Data    1   Oracle  Infra
D://Data    1   Oracle  Infra
D://Data    1   Dell    Infra

The issue has been resolved.

The problem was I was creating the pattern r = pattern(keys) for each keyword (remind you i had 600 keywords) for each cell of the excel which was completely unnecessary.

So if i had a 10(rows) * 10 (columns) excel file, the same 600 pattern was getting created 600 * 10 * 10 times.

So I created one dictionary of patterns just once and used the same 10 * 10 times.

for cat,key in keyword_dic.items():
    temp = [pattern(keyword) for keyword in key]
    list_of_patterns[cat] = temp

Thanks everyone, for taking the time to go through the post!


Find byte[] in byte[] with Java I made a search method because I want to use binary data like PNG files. The point of this is to be able to find strings like IEND in a PNG file. I think it works properly. Any improvements/fixes would be appreciated.

public static int findString(byte[] in, byte[] find) {
        boolean done = false;
        for(int i = 0; i < in.length; i++) {
            if(in[i] == find[0]) {
                for(int ii = 1; ii < find.length; i++) {
                    if(in[i+ii] != find[ii]) break;
                    else if(ii==find.length-1) done = true;
                }
                if(done) return i-1;
            }
        }
        return -1;
    }

If you want to look for a particular chunk of bytes in a PNG file, don't scan it naïvely byte by byte.  Not only would it be slow, you could also accidentally match some data that happens to look like a marker.

Instead, you should take advantage of the chunk layout described in the PNG specification.  Each chunk is tagged with a type and a length.  If the tag is of a type that you are not interested in, seek ahead to the next chunk — the length field will tell you how many bytes to skip.


Method to Return a Hibernate Session Can this method be improved?:

public static Session getHibernateSession() throws SQLException{
    synchronized(dbSessionMutex){
        if(session == null || !session.isOpen()){
            session = HibernateUtil.getSessionFactory().openSession();
        }
        else{
            final SessionFactoryImplementor sessionFactoryImplementation = (SessionFactoryImplementor) session.getSessionFactory();
            final ConnectionProvider connectionProvider = sessionFactoryImplementation.getConnectionProvider();
            try{
                final Connection conn = connectionProvider.getConnection();
                if(conn.isClosed()){
                    session.close();
                    session = HibernateUtil.getSessionFactory().openSession();
                }
                else{
                    conn.close();
                }
            }
            catch(Exception e){
                LOGGER.error("Exception: ", e);
                session.close();
                session = HibernateUtil.getSessionFactory().openSession();
            }
        }
    }
    return session;
}

No, this method cannot be improved because it is completely wrong I am afraid.

Fact: Sessions are not thread safe, don't try make them thread safe, because you don't know how to do it and there are reasons why they are not thread safe.You can simply return the Session from the factory like this

public static Session getHibernateSession() throws SQLException{
   Session session = HibernateUtil.getSessionFactory().openSession();
   return session;
}

The main problem in your code is this

 catch(Exception e){
            LOGGER.error("Exception: ", e);
            session.close();
            session = HibernateUtil.getSessionFactory().openSession();
  }

What you trying to do here? Forcing the code to get a connection?You can't do that. If you fail to open a connection first time, I doubt you will get it in 2 nanoseconds time.
If you don't want to create too many instances, then you should let your IoC container manage it for you. You don't have an IoC container? Then get a Session object every time you need it and don't forget to close it when you are done

  try(Session session = getHibernateSession()){
   // use it
  }


Compare the area of two rectangles This is my solution for a problem in my workbook. It says to create a program that accepts input from a user to compare the areas of two rectangles. Is it good or bad practice to call functions in a constructor as soon as it is initialized?

Such as this:

public Rectangle() {
System.out.println("New rectangle dimensions \n" + "---------------
---------------------------");
userInputDimensions();
calculateArea();
}

Or is it better to just call the functions from the main class and keep the constructor empty? Everything works, I'm just trying to get some general advice on how to improve this code.

Main Class

public class Main {

public static void main(String[] args) {

Rectangle rectangle1 = new Rectangle();
Rectangle rectangle2 = new Rectangle();
Rectangle.CompareAreasOfRectangles test = rectangle1.new
CompareAreasOfRectangles(rectangle1, rectangle2);

}
}

Rectangle Class

import java.util.*;

public class Rectangle {
private double rectangleWidth;
private double rectangleLength;
private double areaOfRectangle;

public Rectangle() {
    System.out.println("New rectangle dimensions \n" + "---------------
---------------------------");
    userInputDimensions();
    calculateArea();
}

//Get Dimensions of Rectangle

private void userInputDimensions() {
    this.rectangleWidth = this.getPositiveInput("Please enter the width
of the rectangle");
    this.rectangleLength = this.getPositiveInput("Please enter the
length of the rectangle");
}

private double getPositiveInput(String prompt) {
    Scanner reader = new Scanner(System.in);
    double result;
    do {
        System.out.println(prompt);
        result = reader.nextDouble();
        if (result <= 0) {
            System.out.println("Enter a number greater than zero.");
        }
    } while (result <= 0);
    return result;
}

private void calculateArea() {
    this.areaOfRectangle = rectangleWidth * rectangleLength;
}

private double getAreaOfRectangle() {
    return areaOfRectangle;
}

// Compare areas of Rectangles

public class CompareAreasOfRectangles {
private Rectangle rectangle1;
private Rectangle rectangle2;

public CompareAreasOfRectangles(Rectangle rectangle1, Rectangle
rectangle2) {
    this.rectangle1 = rectangle1;
    this.rectangle2 = rectangle2;

    if (rectangle1.getAreaOfRectangle() >
rectangle2.getAreaOfRectangle()) {
        System.out.println("The area of Rectangle 1 is greater than the
area of Rectangle 2");
    } else if (rectangle1.getAreaOfRectangle() <
rectangle2.getAreaOfRectangle()) {
        System.out.println("The area of Rectangle 2 is greater than the
area of Rectangle 1");
    } else {
        System.out.println("The areas are equal");
    }
}
}
}

It is always a best practice to use constructor only for initializing default values. Whatever logic or the function which you want to use is good to call directly from main method or separate method.


Loan calculator in C I wanted to create a loan calculator, but the code I could think of was using 3 functions, and calling them and getting result. But I felt it was easy but bad code because I thought this can be written in fewer lines and with more quality. I wanted to ask what are some things to keep in my mind while trying to write better code instead of going in flow and creating many functions.

#include<stdio.h>
#include<stdlib.h>
#include<math.h>

float loanAmount,interestRate,Discount,i,loanPayment; //global variables

int years;

void loanDetails(void); //input from user regarding the loan
float annualRate(void);// interest rate
float discount(void); //discount to be added

int main() //main
{

    loanDetails(); //called three functions
    annualRate();
    discount();

    loanPayment = (loanAmount)/(Discount); //formula for monthly payments

    printf("\nthe monthly loan payment is %f",loanPayment);

    return EXIT_SUCCESS;
}

void loanDetails(void) //taking input from user
{
    printf("please enter Total Loan Amount: ");
    scanf("%f",&loanAmount);
    printf("please enter the number of years: ");
    scanf("%d",&years);
    printf("please enter the annualrate to be applied: ");
    scanf("%f",&interestRate);
}

float annualRate(void) //calculated annual rate
{
    float *ann;
    ann = &interestRate;
    i = ((*ann)/(100))/12;
    printf("the value for %d years is %f",years,i);
    return i;
}

float discount(void) //calculated dicount
{
    float *x,y;
    x = &i;
    y = 1 + *x;
    int n = years*12;

    float topD = (pow((y),n)-1);
    float botD = (y-1)*pow((y),n);
    Discount = topD/botD;
    printf("\nthe value of discount is : %f",Discount);
    return Discount;
}

There are a few things to note here. There is extensive referencing and dereferencing of variable addresses, which makes the code less clear. For example, float *x,y; x = &i; y = 1 + *x;.  This is unnecessary and seems imposed by thinking related to your use of global variables.

There is nothing wrong with passing pointers as arguments to functions, rather than using global variables.

There is also nothing wrong with using functions. The code below is clearer. Additionally, it removes the need for the function "annualRate," as that is simply a division.

#include<stdio.h>
#include<stdlib.h>
#include<math.h>

void loanDetails(float * loanAmount, int * years, float* interestRate);
float discount(int years, float i);

int main (void) {
    float loanAmount, interestRate, disc;
    int years;

    loanDetails(&loanAmount, &years, &interestRate);
    disc = discount(years, interestRate/1200);

    printf("\nthe value of discount is : %f",disc);

    printf("\nthe monthly loan payment is %f",loanAmount/disc);
    return EXIT_SUCCESS;
}

void loanDetails(float * loanAmount, int * years, float* interestRate) {
    printf("please enter Total Loan Amount: ");
    scanf("%f",loanAmount);
    printf("please enter the number of years: ");
    scanf("%d",years);
    printf("please enter the annualrate to be applied: ");
    scanf("%f",interestRate);
}
float discount(int years, float i) {
    int months = years*12;
    return (pow((1+i),months)-1)/((i)*pow((1+i),months));
}

If you wanted no functions other than main, you could do the following. Essentially, move the code from the functions to main. This is appropriste, considering that the code is not repeated, runs sequentially, and does not have varied/multiple inputs.

#include<stdio.h>
#include<stdlib.h>
#include<math.h>

int main (void) {
    float loanAmount, interestRate, disc, i;
    int years, months;

    printf("please enter Total Loan Amount: ");
    scanf("%f",&loanAmount);
    printf("please enter the number of years: ");
    scanf("%d",&years);
    printf("please enter the annualrate to be applied: ");
    scanf("%f",&interestRate);

    months = years*12;
    i = interestRate/1200;

    disc = (pow((1+i),months)-1)/((i)*pow((1+i),months));

    printf("\nthe value of discount is : %f",disc);

    printf("\nthe monthly loan payment is %f",loanAmount/disc);
    return EXIT_SUCCESS;
}


Simple Capital City Quiz in Python3 I've written this very simple quiz program, I'm trying to get to grips with OOP (Object Orientated Programming), so I've started simple with very basic Classes. The nature of the code would have suited using functions but I wanted to practice with Python Classes.

Any comments or suggestions about my code would be most welcome, my ultimate goal is to be as pythonic as possible in future projects.

I've tried to follow Python PEP8 guidelines.

from colorama import *
import random
import os

init()

class StartQuiz():
    def __init__(self, capital_dict: str = {}, randomize_keys: str = []):
        self.capital_dict = capital_dict
        self.randomize_keys = randomize_keys

    def set_up(self):
        with open('capitals.txt') as file:
            for data in file:
                [country, capital] = data.split(',')
                self.capital_dict[country] = capital.strip()
            self.randomize_keys = list(self.capital_dict)
            random.shuffle(self.randomize_keys)
            return [self.capital_dict, self.randomize_keys]

class DisplayAnswer(StartQuiz):
    def __init__(self, capital_dict, randomize_keys):
        super().__init__(capital_dict, randomize_keys)

    def show_answer(self):
        for city in self.randomize_keys:
            print(Fore.BLUE + f'{self.capital_dict[city]}', end='  ')
        print('\n')

class DisplayQuestion(StartQuiz):
    score: int = 0
    def __init__(self, capital_dict, randomize_keys):
        super().__init__(capital_dict, randomize_keys)

    def ask_question(self):
        for country in self.randomize_keys:
            print(Fore.YELLOW + f'Capital of {country} is ? ', end='')
            answer = input(Fore.WHITE)
            if answer.lower() == self.capital_dict[country].lower():
                print(Fore.BLUE + 'Correct!')
                self.score += 1
            else:
                print(Fore.RED + 'Incorrect!')
        return [self.score, len(self.capital_dict)]

def main():

    startQuiz = StartQuiz()
    while True:
        os.system('CLS')
        print(Fore.WHITE + '***  Capital City Quiz  ***\n')
        print('Choose the correct city from the list below:\n')

        DisplayAnswer(startQuiz.set_up()[0], startQuiz.set_up()[1]).show_answer()
        total_score = DisplayQuestion(startQuiz.set_up()[0], startQuiz.set_up()[1]).ask_question()

        print(Fore.WHITE + f'\nYou scored: {total_score[0]} out of {total_score[1]}')
        input('\nPress <Enter> to play again')

if __name__ == "__main__":
    main()

The classes' hierarchy and their names don't make much sense to me. Ideally, a class should represent one well-defined entity. What kind of entity is StartQuiz? The name is confusing and there's no documentation to clarify it. I'd recommend naming it properly (something like QuestionsLoader) and appropriate doc comments.

It also mixes up reading the input and choosing random questions. These are different things so they belong to different methods (and possibly even different classes).

The DisplayAnswer is also a very strange class name (it's something like  OutputFormatter, isn't it?). I ask the same question again: "What kind of entities do its instance represent?" and I can't really answer it. The inheritance doesn't make any sense here. Inheritance represent "is-a" relation. Is a DisplayAnswer a StartQuiz? With such names, the question sounds very weird, but even with proper renaming the answer is clearly no (I don't see any rationale behind an affirmative answer here).

The same applies to the DisplayQuestion class.

There's one more thing: there's pretty much no point in having a class that has one method that prints something to the output and doesn't encapsulate any state. You're even using it as a function: DisplayAnswer(startQuiz.set_up()[0], startQuiz.set_up()[1]).show_answer(). It could be just a function show_answer, couldn't it? The situation with the DisplayQuestion is the same here.

Even if you want to keep them all as classes, I'd strongly recommend taking a step back and redesigning your system. Having classes for the sake of having classes makes no sense. There should be some kind of "physical" (or "real world") meaning to them. You can start with the following questions:

What kind of entities are there in this problem? Typically, an entity can and should be described with a noun like loader, formatter and so on.
Once you know what these entities are, you can think of the classes to implement them (but again, the "physical" meaning comes first. If something is just one simple stateless action, I'd rather make it a function).
This part also includes designing the relations between the classes. Inheritance means "is-a". Before building a hierarchy, ask yourself: "Is X really Y?").

There're a lot of other issues with this code (hardcoded file names, improper default arguments use, lack of documentation and tests and so on), but I think that fixing the high-level design is the first thing to do right now. It's impossible to write good code without modeling the problem properly, so that comes first.


Searching local drive for multiple files Well, I'm writing a piece of code that will take a list of file names and should search a local drive for each file, determine if it's there or not, and return an array of all the paths found.

Right now I'm using a function that searches for one file and I call this function for each file in the list:

Public Function DriveSearch(ByVal sDir As String, sFile As String) As String()
    Dim filesFound As New List(Of String)
    Try
        For Each dir As String In Directory.GetDirectories(sDir)
            For Each file In Directory.GetFiles(dir, sFile)
                filesFound.Add(file)
            Next
            filesFound.AddRange(DriveSearch(dir, sFile))
        Next
    Catch ex As Exception
        Console.WriteLine(ex.Message)
    End Try
    Return filesFound.ToArray
End Function

But I'm wondering if there's a faster way to achieve this. Maybe I should index all the files in the drive first before searching? (I'm not sure how to do this yet).

The number of files in the list is around 100 (for now) but it's likely to increase to couple thousands.

Edit:

I decided that I won't need the duplicates, so here's my current function:

Public Function DriveSearch(ByVal sDir As String, sFile As String) As String
    Dim filePath As String = String.Empty
    Try
        Dim arrFiles As String() = Directory.GetFiles(sDir, sFile)
        If arrFiles.Length > 0 Then
            filePath = arrFiles(0)
        Else
            For Each dir As String In Directory.GetDirectories(sDir)
                filePath = DriveSearch(dir, sFile)
                If filePath.Length > 0 Then Exit For
            Next
        End If
    Catch ex As Exception
        Console.WriteLine(ex.Message)
    End Try
    Return filePath
End Function

Still not sure if this is the fastest way to search for 100+ files.

This seems to be faster by 5 seconds on 12:

Public Function DriveSearch(directory As String, pattern As String) As String()
    Dim filesFound As List(Of String) = Nothing

    Try
        filesFound = GetFilesRecurively(directory, pattern)
    Catch ex As Exception
        Console.WriteLine(ex.Message)
    End Try

    Return filesFound?.ToArray
End Function

Private Function GetFilesRecurively(directory As String, pattern As String) As List(Of String)
    Dim filesFound = IO.Directory.EnumerateDirectories(directory) _
                                 .AsParallel() _
                                 .SelectMany(Function(subDirectory) GetFilesRecurively(subDirectory, pattern)) _
                                 .ToList()

    filesFound.AddRange(IO.Directory.EnumerateFiles(directory, pattern))

    Return filesFound
End Function

This version beats them all taking only few milliseconds:

Public Function DriveSearch(directory As String, pattern As String) As String()
    Dim filesFound As List(Of String) = Nothing

    Try
        filesFound = IO.Directory.EnumerateFiles(directory, pattern, SearchOption.AllDirectories).ToList()
    Catch ex As Exception
        Console.WriteLine(ex.Message)
    End Try

    Return filesFound?.ToArray
End Function

You can also validate the arguments and save yourself a Try..Catch:

Public Function DriveSearch(path As String, pattern As String) As IEnumerable(Of String)
    Return If _
        (
            ArgumentsValid(path, pattern),
            Directory.EnumerateFiles(path, pattern, SearchOption.AllDirectories),
            Enumerable.Empty(Of String)
        )
End Function

Private Function ArgumentsValid(path As String, pattern As String) As Boolean
    If Not Directory.Exists(path) Then
        Console.WriteLine("Invalid path")
        Return False
    End If

    If pattern Is Nothing Then
        Console.WriteLine("Invalid pattern")
        Return False
    End If

    Return True
End Function


Speeding up subset sum implementation Important fact: the number of the input is greater than 1.  How do I use this fact to speed up this solution?

public class subSetSumR{
// Solving Subset sum using recursion

public static boolean subSetSumRecur(int [] mySet, int n, int goal){
   if (goal ==0){
         return true;}
   if ((goal<0)|(n>=mySet.length))
      return false;
   if (subSetSumRecur(mySet,n+1,goal - mySet[n])){
      System.out.print(mySet[n]+" ");
      return true;}
   if (subSetSumRecur(mySet,n+1,goal))
      return true;
   return false;
}

At times to speed up a recursion you can use memoization to cache the results of your previous known calculations, trading memory for speed. (Memoization is slightly different to dynamic programming.)

  A memoized function "remembers" the results corresponding to some set
  of specific inputs. Subsequent calls with remembered inputs return the
  remembered result rather than recalculating it, thus eliminating the
  primary cost of a call with given parameters from all but the first
  call made to the function with those parameters.

Here's an example, after I took the liberty to simplify the number of arguments a bit for sake of clarity,

public static Map<List<Integer>, Boolean> cache = new HashMap<List<Integer>, Boolean>();

public static boolean subSetSum(List<Integer> S, int sum) {
    if (sum == 0) return true;
    if (S.size() == 0 || sum < 0) return false;

    List<Integer> key = new ArrayList<Integer>(S); // composite (set, sum) memoization key
    key.add(sum);
    // let x be the first element of S, then there is either a solution with
    // the set S - {x} without the first element considered, or there is a
    // solution where x is accounted for, ie with the set S - {x} and sum - x
    if (!cache.containsKey(key)) {
        List<Integer> forwardSet = S.subList(1, S.size());
        cache.put(key, subSetSum(forwardSet, sum) || subSetSum(forwardSet, sum - S.get(0)));
    }
    return cache.get(key);
}
// Example: subSetSum(Arrays.asList(new Integer[] { -7, -3, -2, 5, 8 }), 1);

Note the caveat that the example outlines the trade-off described above, it may or may not be faster than your specific example. For example, notice that the list of integers that constitutes the key is cloned, which is suboptimal.


Intersection of Array Question, limited only to methods Problem: Find the intersection of two Object arrays without creating any new classes (no anonymous classes either) and without using any native Java classes in the solution (so no ArrayList  implementations etc., but Object can be used)

My solution:

public Object[] handleArrayIntersection(Object[] array1, Object[] array2) {
    if(array1.length == 0 || array2.length == 0){
        return new Object[0];
    }
    // the maximum possible number of intersections is the smaller array's size
    int maxPossibleLength = array1.length < array2.length ? array1.length : array2.length;
    Object[] intersectionValues = new Object[maxPossibleLength];
    int itemCount = 0;
    for(int i = 0; i < array1.length; i++){
        Object arr1Val = array1[i];
        if(contains(array2, arr1Val) && !contains(intersectionValues, arr1Val)){
            intersectionValues[itemCount] = arr1Val;
            itemCount++;
        }
    }

    // reduce the array down to only contain the intersections
    Object[] intersection = new Object[itemCount];
    for(int i = 0; i < itemCount; i++){
        intersection[i] = intersectionValues[i];
    }
    return intersection;
}

public boolean contains(Object[] newArr, Object arr1Val) {
        boolean contains = false;
        for(int i = 0; i < newArr.length; i++){
            Object val = newArr[i];
            if((val == null && arr1Val == null) || (val != null && val.equals(arr1Val))){
                contains = true;
                break;
            }
        }
        return contains;
}

I haven't tried to solve this type of problem for awhile and I am really curious what people  think of this solution in terms of efficiency, robustness and readability. I arrived at this solution by using TDD (Test Driven Development), but I'm not including the test because I'm more interested in people reviewing the solution.

If you want to optimize you can : Use arrayCopy() from the System Class to create the final array and reduce variable. Here is my "solution" :

public static Object[] handleArrayIntersection(Object[] array1, Object[] array2) {
    Object[] dest;

    int itemCount = 0;
    Object[] intersect = new Object[array1.length < array2.length ? array1.length : array2.length];

    for(int i = 0; i < array1.length; i++) {
        Object arr1Val = array1[i];
        if(contains(array2, arr1Val) && !contains(intersect, arr1Val)) {
            intersect[itemCount++] = arr1Val;
        }
    }

    dest = new Object[itemCount];
    if(itemCount > 0) {
        System.arraycopy(intersect, 0, dest, 0, itemCount);
    }

    return dest;
}


Object in JavaScript, superfluous? I've got a JavaScript object and I am not sure how to implement the last part: adding up all of the prices and putting that in the last row of the table. Besides this question, I would also like to ask you to look at the code and advice me if I can make the code slimmer, less lines of code.

This is the code I would like reviewed:

    function ShoppingList(){
    this.items = [];
    this.setItem = function(product, price){
        var item = new Item(product, price);
        this.items.push(item);
    };
    this.render = function(){
        var placeHolder = document.getElementById("shoppingList");
        placeHolder.innerHTML = "";
        var messageDiv = document.getElementById("message");
        messageDiv.innerHTML = "";

        var tr = document.createElement("tr");
        tr.id = "header";

        var thProduct = document.createElement("th");
        thProduct.innerHTML = "Product";
        tr.appendChild(thProduct);

        var thPrice = document.createElement("th");
        thPrice.innerHTML = "Prijs";
        tr.appendChild(thPrice);

        var thDel = document.createElement("th");
        thDel.innerHTML = "Verwijder";
        tr.appendChild(thDel);

        placeHolder.appendChild(tr);

        for(var i = 0; i < this.items.length; i++){
            var tr = document.createElement("tr");
            tr.id = i;

            var tdProduct = document.createElement("td");
            tdProduct.innerHTML = this.items[i].product;
            tr.appendChild(tdProduct);

            var tdPrice = document.createElement("td");
            tdPrice.innerHTML = Number(this.items[i].price);
            tr.appendChild(tdPrice);

            var tdDel = document.createElement("td");
            tdDel.innerHTML = "Verwijder";
            tdDel.addEventListener("click", delItem, false);
            tr.appendChild(tdDel);

            placeHolder.appendChild(tr);
        }
    };
}

So, in short: how can I write a new table row to the table with the total price that updates when table rows are added or removed and how can I slim down my code?

I usually make some simple helper functions to simplify and shorten the code building html. (As your programme gets more complicated you can expand these to suit your needs, or if you prefer, switch to a library like jquery.) In the version below I have also added a few lines to add a total row to the table. However, think about whether you need to rewrite the entire table each time it changes, or could simply add or delete individual rows. If the latter, you would want something like addRow, deleteRow and updateTotal methods.

function build(parent, type, innerHTML) {
    // Make a new element, optionally fill it with html, append it to the parent,
    // and return it.
    var el = document.createElement(type);
    if (innerHTML) {
        el.innerHTML = innerHTML;
    }
    parent.appendChild(el);
    return el;
}

function E(id) {
    // This function has no purpose except to save typing
    return document.getElementById(id);
}

function ShoppingList() {
    var totalCell;
    this.items = [];
    this.setItem = function (product, price) {
        var item = new Item(product, price);
        this.items.push(item);
    };
    this.render = function () {
        E('shoppingList').innerHTML = "";
        E('message').innerHTML = "";
        var tr = build(E('shoppingList'), 'tr');
        tr.id = 'header';
        build(tr, 'th', 'Product');
        build(tr, 'th', 'Prijs');
        build(tr, 'th', 'Verwijder');
        var totalPrice = 0;
        for (var i = 0; i < this.items.length; i++) {
            tr = build(E('shoppingList'), 'tr');
            tr.id = i;
            build(tr, 'td', this.items[i].product);
            build(tr, 'td', this.items[i].price);
            build(tr, 'td', 'Verwijder').addEventListener('click', delItem, false);
            totalPrice += this.items[i].price;
        }
        tr = build(E('shoppingList'), 'tr');
        build(tr, 'td', 'Total');
        totalCell = build(tr, 'td', totalPrice);
        build(tr, 'td', '');
    };
}


GroupBy field with and without white spaces I have these invoices listed:

def invoices = [
'LEDES98BI V2',
'LINE|INVOICE_DATE|INVOICE_NUMBER|INVOICE_TOTAL',
'1|20150301|INV-Error_Test1|22',
'2|20150301|INV-Error_Test1|24',
'3|20150301|INV-Error_Test2|26',
'4|20150301|INV-Error_Test2|28,']

I am trying to do groupBy on the above collection with INVOICE_NUMBER and trying to achieve map with INVOICE_NUMBER and lines as values:

def lines = invoices*.split('\\|').findAll{ it.size()>1 }
def heads = lines.first()
def invoiceMap =  lines.tail().collect{ [heads, it].transpose().collectEntries() }.groupBy{ it.INVOICE_NUMBER }

If I print invoiceMap I get what I intended as this map:

 [INV-Error_Test1:[[LINE:1, INVOICE_DATE:20150301, INVOICE_NUMBER:INV-Error_Test1, INVOICE_TOTAL:22],
                   [LINE:2, INVOICE_DATE:20150301, INVOICE_NUMBER:INV-Error_Test1, INVOICE_TOTAL:24]],
  INV-Error_Test2:[[LINE:3, INVOICE_DATE:20150301, INVOICE_NUMBER:INV-Error_Test2, INVOICE_TOTAL:26],
                   [LINE:4, INVOICE_DATE:20150301, INVOICE_NUMBER:INV-Error_Test2, INVOICE_TOTAL:28,]]
  ]

But if the INVOICE_NUMBER has any white spaces with it in the invoices map my code doesn't work, so I tweaked the code as below to get it working:

 def invLineDetails = lines.tail().collect{ [heads, it].transpose().collectEntries() }.groupBy{it[it.keySet().find{it.contains('INVOICE_NUMBER')}] }

Can this code be optimized/refactored further?

I could not produce an error when an invoice number contains white spaces. I made some minor tweaks.

def invoices = [
'LEDES98BI V2',
'LINE|INVOICE_DATE|INVOICE_NUMBER|INVOICE_TOTAL',
'1|20150301|INV-Error_Test1|22',
'2|20150301|INV-Error_Test1|24',
'3|20150301|INV-Error_Test2|26',
'4|20150301|INV-Error_Test2|28,',
'5|20150301|INV WITH WHITESPACE|29,']

def lines = invoices
    .findAll { it.contains('|') }
    .collect { it.tokenize('|') }

def headers = lines.first()
def invoiceMap =  lines.tail().collect{
    [headers, it].transpose().collectEntries()
}.groupBy{ it.INVOICE_NUMBER }


Closing Tab Item Confirmation For this project I'm targeting .NET 4.6 with Prism 6 as a toolkit to implement correct MVVM patterns.

The code below allows me to raise a dialog box with Yes, No, Cancel buttons. If the user clicks 'Yes' the content is saved then the tab item closed. If 'No' the tab item is only closed, and if 'Cancelled' then return, do not close anything.

However the dialog is only displayed if the content is dirty. If not just close it.

I feel like in its current sate it's very messy and could be cleaned up. Take a look below and tell me what you think.

internal class CloseTabAction : TriggerAction<Button>
{
    protected override void Invoke(object parameter)
    {
        var args = parameter as RoutedEventArgs;
        if (args == null) return;

        var tabItem = FindParent<TabItem>(args.OriginalSource as DependencyObject);
        if (tabItem == null) return;

        var tabControl = FindParent<TabControl>(tabItem);
        if (tabControl == null) return;

        var region = RegionManager.GetObservableRegion(tabControl).Value;
        if (region == null) return;

        var context = new NavigationContext(region.NavigationService, null);

        if (Implements<IRequestTabItemClose>(tabItem.Content, r => r.IsContentDirty(isDirty =>
        {
            if (isDirty)
            {
                if (Implements<IRequestTabItemClose>(tabItem.Content,
                    i => i.ConfirmCloseRequest((canSave, cancel) =>
                    {
                        if (cancel) return;
                        if (!region.Views.Contains(tabItem.Content)) return;
                        if (canSave)
                        {
                            i.SaveContent();
                            region.Remove(tabItem.Content);
                            return;
                        }
                        region.Remove(tabItem.Content);
                    })))
                    return;
            }
            region.Remove(tabItem.Content);
        }))) return;
    }

    private static T FindParent<T>(DependencyObject child) where T : DependencyObject
    {
        while (true)
        {
            var parentObject = VisualTreeHelper.GetParent(child);

            if (parentObject == null) return null;

            var parent = parentObject as T;
            if (parent != null) return parent;

            child = parentObject;
        }
    }

    private static T Implementor<T>(object content) where T : class
    {
        T implementor = content as T;
        if (implementor != null) return implementor;

        var element = content as FrameworkElement;
        if (element != null) implementor = element.DataContext as T;
        return implementor;
    }

    private static bool Implements<T>(object content, Action<T> action) where T : class
    {
        T target = Implementor<T>(content);
        if (target == null) return false;
        action(target);
        return true;
    }
}

Here are my custom interfaces:

public interface IRequestTabItemClose
{
    void ConfirmCloseRequest(Action<bool, bool> callback);
    void SaveContent();
    void IsContentDirty(Action<bool> callBack);
}

public interface ICloseConfirmation : INotification
{
    bool Confirmed { get; set; }
    bool Cancel { get; set; }
}

ViewModel:

    public bool IsDirty { get; set; }

    public virtual void ConfirmCloseRequest(Action<bool, bool> callback)
    {
        ClosePopupRequest.Raise(
            new CloseConfirmation {Title = "Confirm", Content = "Do you want to save this view before closing?"},
            r => { callback(r.Confirmed, r.Cancel); });
    }

    public virtual void SaveContent()
    {
        // Save stuff
    }

    public void IsContentDirty(Action<bool> callBack)
    {
        callBack(IsDirty);
    }

I've cleaned up my CloseTabAction class see below:

I took out the method 'Implements' because the interface is attached to my BaseViewModel so I know that every VM will have it, no need to check.

I created a new method to check if the content is dirty. Rather than putting it in my Invoke method. From there I raise my dialog and get the results back.

It still may need some work, but it's much better now.

internal class CloseTabAction : TriggerAction<Button>
{
    protected override void Invoke(object parameter)
    {
        var args = parameter as RoutedEventArgs;
        if (args == null) return;

        var tabItem = FindParent<TabItem>(args.OriginalSource as DependencyObject);
        if (tabItem == null) return;

        var tabControl = FindParent<TabControl>(tabItem);
        if (tabControl == null) return;

        var region = RegionManager.GetObservableRegion(tabControl).Value;
        if (region == null) return;

        var content = Implementor<IRequestTabItemClose>(tabItem.Content);

        if (IsDirty(content))
        {
            content?.ConfirmCloseRequest((canSave, cancel) =>
            {
                if (cancel) return;
                if (canSave)
                {
                    content.SaveContent();
                    region.Remove(tabItem.Content);
                    return;
                }
                region.Remove(tabItem.Content);
            });
        }
        else
        {
            region.Remove(tabItem.Content);
        }
    }

    private static bool IsDirty(IRequestTabItemClose content)
    {
        var isContentDirty = false;

        content?.IsContentDirty(isDirty =>
        {
            if (isDirty)
                isContentDirty = true;
        });
        return isContentDirty;
    }

    private static T FindParent<T>(DependencyObject child) where T : DependencyObject
    {
        while (true)
        {
            var parentObject = VisualTreeHelper.GetParent(child);

            if (parentObject == null) return null;

            var parent = parentObject as T;
            if (parent != null) return parent;

            child = parentObject;
        }
    }

    private static T Implementor<T>(object content) where T : class
    {
        T implementor = content as T;
        if (implementor != null) return implementor;

        var element = content as FrameworkElement;
        if (element != null) implementor = element.DataContext as T;
        return implementor;
    }
}


Scrubbing user input I'm taking user input from a file in form (1, 2, 3) to create a color, and I just wanted to know if I was taking most cases into account, and if there is any way I could improve this method in terms of being robust and readable.

private Color getColorFromString(String rgb) throws MalformedColorException{
    rgb = rgb.replaceAll("[()\\s]", "");
    String[] colors = rgb.split(",");
    if(colors.length > 3){
        throw new MalformedColorException("Too many arguments");
    }else if (colors.length < 3){
        throw new MalformedColorException("Too little arguments");
    }
    try{
        int red = Integer.parseInt(colors[0]);
        int green = Integer.parseInt(colors[1]);
        int blue = Integer.parseInt(colors[2]);

        return new Color(red, green, blue);
    }catch(NumberFormatException e){
        throw new MalformedColorException("Malformed number", e);
    }
}

I only have a few minor suggestions, but none are hard-line.

You have a space between the else if and open parenthesis. I don't like this style of removing spaces around keywords and from between ) and { as it becomes hard to read without syntax highlighting (even with it).
Instead of too few/many arguments which isn't very specific, have a single exception that says how many were provided and how many are required.

if (colors.length != 3) {
    throw new MalformedColorException("Color requires 3 values, got " + colors.length);
}

Moving the individual value parsing to a separate method would allow for more checks (range, sign, etc) and allow specifying which color was incorrect in the exception.

return new Color(
    parseComponent("red", colors[0]),
    parseComponent("green", colors[1]),
    parseComponent("blue", colors[2])
);

private int parseComponent(String name, String value) {
    try {
        int parsed = Integer.parseInt(value);
        if (parsed < 0 || parsed > 255) {
            throw new MalformedColorException(
                    name + " component \"" + value + "\" out of range");
        }
        return parsed;
    } catch (NumberFormatException e) {
        throw new MalformedColorException(
                name + " component \"" + value + "\" must be an integer");
    }
}


If any image found and bigger than thumbnail settings, get that one Is this an ideal use of if..else or can this code be simplified/beautified further?

    // If any image found and bigger than thumbnail settings, get that one
    if ( isset( $matches ) && !empty( $matches[1][0] ) ) {
        $image_found = $matches[1][0];
        $image_dimensions = getimagesize($image_found);
        if ( ( $image_dimensions[0] > $width ) && ( $image_dimensions[1] > $height ) ) {
            $thumbnail_img = $matches[1][0];
        }
        // Check 2nd image found
        else if ( !empty( $matches[1][1] ) ) {
            $image_found = $matches[1][1];
            $image_dimensions = getimagesize($image_found);
            if ( ( $image_dimensions[0] > $width ) && ( $image_dimensions[1] > $height ) ) {
                $thumbnail_img = $matches[1][1];
            }
            // Check 3rd image found
            else if ( !empty( $matches[1][2] ) ) {
                $image_found = $matches[1][2];
                $image_dimensions = getimagesize($image_found);
                if ( ( $image_dimensions[0] > $width ) && ( $image_dimensions[1] > $height ) ) {
                    $thumbnail_img = $matches[1][2];
                }
            }
        }
    }

Let me make it clear for myself. You have an array of images, which sometimes can be empty. You start off by checking the first image, if it's valid for your needs you take it, and if not you keep on checking the next one, and the next one, and so on until you find the one which is valid. Is that right? If so, here are my few thoughts.

Firstly, I must say that Glenn has made good points, namely: 1) factor the common code in all cases out to a separate function, and 2) if you have nested conditionals, most times it's better and more readable to replace it with Guard Clauses.
However, personally I would prefer the following solution:

$theImg = null;
$i = 0;
while (!empty($matches[1][$i])) {
    $imgAtHand = $matches[1][$i];
    $dimensions = getimagesize($imgAtHand);
    if (($dimensions[0] > $width) && ($dimensions[1] > $height)) {
        $theImg = $imgAtHand;
        break;
    }
    $i++;
}

It has a couple of advantages over the "nested conditional" approach:

it eliminates code duplication;
it will work regardless of the array size.

One more thing. The following piece of code

isset($matches) && !empty($matches[1][0])

may well be replaced with

!empty($matches[1][0])

to make the code less verbose.


Ensure that column names in output retain original variable names, instead of using arbitrary X1, X2, etc Is there a more elegant way to ensure that colnames(output) of a output R dataframe retains the original column variable names in the referenced sample dataset data? So that I can eliminate the 2 gsub() lines of code, which rename my column names back to my original variable names. My R code, which can be run, is as follows

library(mvtnorm)
library(tidyverse)
muvec = apply(data, 2, mean) #data is passed as R dataframe
sigma = cov(data)
df = 10
n = 10
output <- as.data.frame(
        lapply(1:n, function(x) rep(muvec, each=10) + rmvt(10, sigma=sigma, df=df))) %>%
        select (sort(names(.)))
#looped function output assigns arbitrary variable names X1 and X2, instead of original variable names apple and pear
colnames(output) = gsub("X1", "apple", colnames(output))
colnames(output) = gsub("X2", "pear", colnames(output))
print(output)

data as follows

+-------+-------+
| apple | pear  |
+-------+-------+
|  0.18 |  0.05 |
|  0.22 |  0.03 |
|  0.21 |  0.03 |
|  0.11 |     0 |
|  0.15 |  0.03 |
|  0.13 |  0.01 |
|  0.03 |  0.01 |
|     0 |     0 |
| -0.06 | -0.01 |
| -0.09 | -0.02 |
+-------+-------+

The most efficient way that I can think of is creating a function with rmvt which takes an additional argument to decide whether column names should be assigned based on the input (i.e. sigma) or not. If TRUE, before returning the results of rmvt function, it assigns the original names. See below (I used airquality dataset to make a reproducible example);

library(mvtnorm)
library(dplyr)

dat <- na.omit(datasets::airquality)[1:2,1:2]

muvec = apply(dat, 2, mean) #data is passed as R dataframe
sigma = cov(dat)
df = 10
n = 2

rmvt.names <- function(n, sigma = diag(2), df = 1, delta = rep(0, nrow(sigma)),
                       type = c("shifted", "Kshirsagar"), names = FALSE){
  out <- rmvt(n, sigma, df, delta, type)
  if (names) { colnames(out) <- colnames(sigma) }
  return(out)
}

lapply(1:n, function(x)
            rep(muvec, each=10) + rmvt.names(10, sigma=sigma, df=df, names = T)) %>%
  as.data.frame() %>%
  select (sort(names(.)))

#>       Ozone  Ozone.1    Solar.R  Solar.R.1
#> 1  37.47822 45.06226 139.286427 248.496571
#> 2  40.59224 42.78024 184.128243 215.635423
#> 3  33.03107 35.89953  75.247397 116.553211
#> 4  28.07207 28.48342   3.837864   9.761237
#> 5  38.15097 39.46869 148.974039 167.949166
#> 6  44.04143 42.68460 233.796536 214.258203
#> 7  36.54224 40.53313 125.808217 183.277053
#> 8  37.37606 38.77892 137.815314 158.016502
#> 9  43.99939 37.11836 233.191196 134.104332
#> 10 37.63792 32.28113 141.586044  64.448253

Created on 2020-05-06 by the reprex package (v0.3.0)


Multiple 'using' statements in method implementation I'm learning WebAPI framework for ASP.NET and C#. Is the following way of handling incoming data correct?

    [HttpPost]
    [ExpiredCookieCheck]
    public IHttpActionResult Auth([FromBody]AuthenticationData authenticationData)
    {
        try
        {
            if (authenticationData == null)
                using (var responseManager = new ResponseManager())
                    return responseManager.PrepareMessage(
                        HttpStatusCode.Unauthorized,
                        "No authentication model was provided in request body.",
                        null,
                        ResponseManager.ResponseType.InterfacePlainText
                    );

            using (var authenticationHandler = new AuthenticationHandler())
            {
                var validationData = authenticationHandler.CheckAuthenticationDataFromWeb(authenticationData);

                if (!validationData.Key)
                    using (var responseManager = new ResponseManager())
                        return responseManager.PrepareMessage(
                            HttpStatusCode.Unauthorized,
                            "Incorrect credentials for authentication.",
                            null,
                            ResponseManager.ResponseType.InterfacePlainText
                        );

                using (var cookieManager = new CookieManager())
                using (var responseManager = new ResponseManager())
                {
                    var authenticationCookie = cookieManager.CreateAuthenticationCookieHeader(Request.RequestUri.Host);
                    cookieManager.SaveAuthenticationCookie(validationData.Value.Id, authenticationCookie);

                    return responseManager.PrepareMessage(
                        HttpStatusCode.OK,
                        null,
                        authenticationCookie,
                        ResponseManager.ResponseType.InterfacePlainText
                    );
                }
            }
        }
        catch (Exception exception)
        {
            using (var responseManager = new ResponseManager())
                return responseManager.PrepareMessage(
                    HttpStatusCode.Unauthorized,
                    exception.ToString(),
                    null,
                    ResponseManager.ResponseType.InterfacePlainText
                );
        }
    }

I'm thinking that I'm using something not correctly. I dislike my way of defining using statements. Maybe it's miles better to define at the beginning of the method all required using statements and then only call internal methods of objects from those using statements.

Also, is it correct to use try/catch block of code in Web API controller method? I'm thinking that it could be incorrect to handle some possible errors in Web API method, couldn't it?

There is nothing inherently wrong with having multiple using statements. It keeps the lifetime of objects to the minimum which is not a bad thing to do.

Another point is that I'd possibly re-factor the error raising: You always prepare the error message with the same parameters except for the actual message. This could easily be encapsulated in a little helper function. So the refactored code could look like this:

private Response GetUnauthorizedResponse(string message)
{
    using (var responseManager = new ResponseManager())
    {
        return responseManager.PrepareMessage(
            HttpStatusCode.Unauthorized,
            message,
            null,
            ResponseManager.ResponseType.InterfacePlainText
        );
    }
}

[HttpPost]
[ExpiredCookieCheck]
public IHttpActionResult Auth([FromBody]AuthenticationData authenticationData)
{
    try
    {
        if (authenticationData == null)
        {
            return GetUnauthorizedResponse("No authentication model was provided in request body.");
        }

        using (var authenticationHandler = new AuthenticationHandler())
        {
            var validationData = authenticationHandler.CheckAuthenticationDataFromWeb(authenticationData);
            if (!validationData.Key)
            {
                return GetUnauthorizedResponse("Incorrect credentials for authentication.");
            }

            using (var cookieManager = new CookieManager())
            using (var responseManager = new ResponseManager())
            {
                var authenticationCookie = cookieManager.CreateAuthenticationCookieHeader(Request.RequestUri.Host);
                cookieManager.SaveAuthenticationCookie(validationData.Value.Id, authenticationCookie);

                return responseManager.PrepareMessage(
                    HttpStatusCode.OK,
                    null,
                    authenticationCookie,
                    ResponseManager.ResponseType.InterfacePlainText
                );
            }
        }
    }
    catch (Exception exception)
    {
        return GetUnauthorizedResponse(exception.ToString());
    }
}

This removed some distracting clutter and lets the reader focus better on the actual functionality of the method.


Implementation of Javascript Classes and Inheritance I've created a simple Javascript library to be able to create "classes" and extend them defining instance and static methods and variables.

It consists in a simple function Class with two methods: create and extend.

var Class = function(){};

Class.extend = function(obj){

    var Extended = function(){};

    for(var key in this.prototype){
        Extended.prototype[key] = this.prototype[key];
    }

    for(var key in obj){
        Extended.prototype[key] = obj[key];
    };

    Extended.prototype.constructor = Extended;

  for (var key in this) {
    Extended[key] = this[key];
  };

  return Extended;
};

Class.create = function(constructor){
    var created = new this();
    for(var key in constructor){
        created[key] = constructor[key];
    }
    return created;
};

Here there's an example of what the library is able to do:

var Model = Class.extend({
    fields: {},
    save: function(){
        return "saving a model at " + Model.url;
    },
    destroy: function(){
        return "destroying...";
    }
});

Model.url = "http://localhost";

Model.all = function(){
    return "all from " + Model.url;
};

Model.find = function(id){
    return "finding by " + id;
};

var User = Model.extend({
    fields: {
        username: "default username",
        password: "default password",
    },
    logout: function(){
        return "logged out " + this.fields.username;
    },
    destroy: function(){
        return this.logout() + " and account destroyed!";
    }
});

User.url = Model.url + "/users";

User.all = function(){
    return "overriding parent! all from " + User.url;
}

User.findByUsername = function(username){
    return "finding by username: " + username;
};

And I've created a js fiddle to test the example  http://jsfiddle.net/R6jGe/

There's no way to call a parent instance method or variable.

how do you think the library could be improved?

I read your code several times, and I could only find few observations:

jQuery has extend as well, the idea of it is sound, you could consider working with arguments and allow for n objects like jQuery does
I am not sure that always overriding the constructor is a good idea in extend
In the same vein, your approach does not allow for custom constructors that take parameters, whereas most OO classes have those ( new Customer( "Bob" ) to give a silly example)
From a lint perspective:

You declare var key thrice in extend, you only need to declare it once, preferably on top
for loop blocks do not need semicolons
You should really look into .hasOwnProperty(), I am pretty sure your code can cause subtle bugs by smushing everything together


How to improve the way I handle greping in this script What my script does is:

Append data to a file on a continuous basis.
If an error pattern is observed, then do something.

I need feedback on the way I am grepping for patterns in the continuously appended file. For grep, I am creating a pattern file first and then using it with -f option to grep. Can I do this better? Any other better way I can write my over all logic.

Here's my script

#!/bin/bash
# List of error patterns to look for. Add more patterns to this list
p1='pattern1'
p2='pattern2'
p3='pattern3'
p4='pattern4'

PATTERNFILE=~/patternfile
FILETOCHECK=~/filetocheck

captureFileToCheck()
{
   //appends data into filetocheck &

}
createPatternFile()
{

    for i in `seq 100` # hopefully 100 is all we will need
    do
        if [ ! "x$p{$i}" = "x" ]
        then
            echo $p{$i} >> $PATTERNFILE
        else
            break
        fi
    done
}

createPatternFile
captureFileToCheck

while [ 1 ]
do

    if grep -f $PATTERNFILE $FILETOCHECK
    then
        echo "something starting"

          #doSometing

        echo "something complete"

        break
    fi

done

Use more quotes, as the saying goes.

I find while true is more clear, but the idiomatic while : is also popular.

You might want to use an array for the patterns, then you can simply loop over it (for pattern in "${patterns[@]}") instead of using seq and the if statement.

Uppercase variables are usually used for exports; that is, stuff which is relevant outside your script.

You should avoid cluttering ~; use tmp_dir="$(mktemp -d)" instead.


Recursively Convert Roman Number to Integer I'm practicing recursive functions and I would be glad to receive some feedback for the following Python code I wrote. I did several tests and the function yielded correct results but I wonder if it can be written more concise or if it can be criticised in some way. Your feedback would help me become better. The functionality of the function is described in the docstring.

def roman(n):
  """Takes a roman number n as an argument
     (roman(n) assumes that the string n is
     correctly written, i.e. it provides no
     error-checking) and returns an integer."""

  #The base-cases:

  if n == "":
    return 0
  elif n == "M":
    return 1000
  elif n == "D":
    return 500
  elif n == "C":
    return 100
  elif n == "L":
    return 50
  elif n == "X":
    return 10
  elif n == "V":
    return 5
  elif n == "I":
    return 1

  #If a smaller number precedes a bigger number,
  #then the smaller number is to be subtracted from
  #the bigger number. Else, it has to be added:

  else:
    if roman(n[0]) < roman(n[1]):
      return (roman(n[1]) - roman(n[0])) + roman(n[2:])
    else:
      return roman(n[0]) + roman(n[1:])

def main(n):
  print(roman(n))

main("MMMCMXCIX")

Writing down all your basecases takes a lot of space. I would instead use a dictionary, which could even be a global constant.
I find using n as an argument for something that is not an integer to be misleading. A generic x or s or roman_numeral would be clearer, IMO.
Python has an official style-guide, PEP8. It recommends using four spaces as indentation.
BASE_CASES = {"": 0, "M": 1000, "D": 500, "C": 100, "L": 50, "X": 10, "V": 5, "I": 1}

def roman(s):
    """Takes a roman number x as an argument and returns an integer.
    Assumes that the string is correctly written, i.e. it provides no error-checking.
    """
    if s in BASE_CASES:
         return BASE_CASES[s]

    # If a smaller number precedes a bigger number,
    # then the smaller number is to be subtracted from
    # the bigger number. Else, it has to be added:
    first, second = map(roman, s[:2])
    if first < second:
        return second - first + roman(s[2:])
    else:
        return first + roman(s[1:])

Of course, using a recursive approach in Python is usually not the best idea, converting it to an iterative approach is much better (i.e. usually faster and not fraught with stack limit issues):
from itertools import tee

def pairwise(iterable):
    "s -> (s0,s1), (s1,s2), (s2, s3), ..."
    a, b = tee(iterable)
    next(b, None)
    return zip(a, b)

def roman(s):
    if s in BASE_CASES:
        return BASE_CASES[s]
    return sum(first if first >= second else second - first
               for first, second in pairwise(map(BASE_CASES.get, s)))


Method to act on parameter or from iterable if not parameters are passed The scenario is as follows. The API of my program will allow the user to perform some method on a particular element (let's say a tuple from a database). If this parameter is not passed to the function, then the "default" behavior is to look for a list of elements (from a database query, for example) and to act on all of them. Like this:

task(simple_object) # will process simple_object
task() # query the database with some previously defined query

And this is what I did (implementation in Python):

@task(query='SELECT * FROM User WHERE age < 18')
def task_disable_account(user):
    disable_user(user)

This way I can call the task_disable_account with one particular User or I can call it with no arguments and then every tuple from the SQL query will be processed. The definition of task decorator is as follows:

task_registry = []
def task(query):
    Task = namedtuple('Task', ['func', 'query'])
    def decorate(func):
        def queried(*_args):
            if not _args:
                map(func, search(query)) # execute on result from SQL
                return
            return func(*_args) # execute on 1 passed parameter
        task_registry.append(Task(queried, query))
        return queried
    return decorate

And this is how I use it (just an example, actual code is more complicated):

def main(): # lets assume a main() function
    for func, _ in task_registry:
        # I'm getting and ID from sys.argv and returning a User object
        # If not passed via arguments, call function with no parameters
        user = get_from_argv()
        if user:
            func(user)
        else:
            func()

The idea behind all this is:

The tasked method should only take care of the processing of 1 element, centralising the way we search for, and how we iterate over them.
Make it easy to add new task methods to our implementation, not having to repeat the boilerplate code over and over.

Not going into the question as to whether or not this is a good idea, I can see very little wrong with the solution.

One thing I would very much like to suggest is the following.

Instead of

map(func, search(query))

please write

for result in search(query):
    func(result)

There are actually some differences:

In Python 3, map builds a iterable, and func(result) will only be called if the iterable is looped over. In this case, this is most definitely not what you want!
In Python 2, map builds a list, and thus evaluates everything. Better, but an intermediate list is now going to be build and thrown away.

In general I would recommend only using map for functions that are side-effect free.

Another thing I'd like to remark is where you build the namedtuple. You do it inside the function, building a new Task class for every task you add. Please move that code outside of the function task.


Crossover and Mutation functions for Genetic Algorithm I'm writing a decryption algorithm using GA with crossover and mutation. My performance is (very) poor, often stagnating early or converging to the incorrect solution. I just want some other people to look at my crossover and mutation methods to see if something is amiss.

Note: crossRate and mutatRate are the crossover and mutation rates respectively and are within the range of [0.00, 1.00]. options is a string of valid genes.

/**
 * method to perform crossover on chromosome
 * @param m - the first parent
 * @param f - the second parent
 * @return - a crossovered chromosome string
 */
private String crossChromosome(String m, String f) {
    StringBuilder chrString = new StringBuilder();
    // if crossover procs
    if (rnd.nextInt(100) < crossRate*100) {
        /**
         * to avoid full reduplication of chromosome
         * set the bound to 2..len-2 otherwise you could
         * get duplication of parent, bypassing crossover entirely
         */
        int crossPoint = rnd.nextInt(m.length()-2)+2;
        // when 2P is used, need a second pivot
        int nextCrossPoint = rnd.nextInt(m.length()-crossPoint)+crossPoint;
        boolean condition;
        for (int i = 0; i < f.length(); i++) { // for each gene
            if (crType == 1) {
                /**
                 * if one point crossover, find one point
                 * in the chromosome where genes before that
                 * point are from parent1 and after, from parent2
                 */
                condition = (i < crossPoint);
            } else {
                /**
                 * if two point, however, find two pivot points
                 * and go F,M,F based on the pivot points
                 */
                condition = (i < crossPoint || i >= nextCrossPoint);
            }
            if (condition) {
                chrString.append(f.charAt(i));
            } else {
                chrString.append(m.charAt(i));
            }
        }
    } else {
        // if no crossover is happening, random parent is used
        return (rnd.nextInt(2) == 1) ? f : m;
    }
    // return the crossovered chromosome
    return chrString.toString();
}

/**
 * method to mutate a chromosome with random genes
 * @param k - the input chromosome
 * @return - the mutated chromosome
 */
private String mutateChromosome(String k) {
    StringBuilder chrString = new StringBuilder();
    for (int i = 0; i < k.length(); i++) { // for each gene
        // if mutation procs
        if (rnd.nextInt(100) < mutatRate*100) {
            // assign a random gene as mutation
            chrString.append(options.charAt(rnd.nextInt(27)));
        } else {
            // otherwise, use verbatim
            chrString.append(k.charAt(i));
        }
    }
    return chrString.toString();
}

There's option of 1-point and 2-point crossover in there.

I rewrote parts of your code obtaining an equivalent version with the focus of relying on char arrays instead of String objects to obtain a simpler code. I noticed you use the number 100 in both methods you posted, it could be preferrable declaring a const value like below:

private static final int BOUND = 100;

Your method crossChromosome is structured like the code below:

private String crossChromosome(String m, String f) {
    if (rnd.nextInt(100) < crossRate*100) {
        //here the body
   }
    else { return (rnd.nextInt(2) == 1) ? f : m; }
    return chrString.toString();
}

You can directly put the else body at the beginning of the method with the negation of the condition like the code below:

private String crossChromosome(String m, String f) {
    if (rnd.nextInt(BOUND) >= crossRate * BOUND) {
        return (rnd.nextInt(2) == 1) ? f : m;
    }

    //here the instructions of the body of your case

    return chrString.toString();
}

You can use char arrays instead of String objects avoiding calls of String methods like the code below:

char[] mArr = m.toCharArray();
char[] fArr = f.toCharArray();
final int mLength = mArr.length;
final int fLength = fArr.length;
final int crossPoint = rnd.nextInt(mLength - 2) + 2;
final int nextCrossPoint = rnd.nextInt(mLength - crossPoint) + crossPoint;

You can rewrite the loop of your method like below:

StringBuilder chrString = new StringBuilder();

for (int i = 0; i < fLength; i++) {
    boolean condition = (i < crossPoint);

    if (crType != 1 && !condition) {
        condition = (i >= nextCrossPoint);
    }
    char ch = condition ? fArr[i] : mArr[i];
    chrString.append(ch);
}

return chrString.toString();

Below the code of your method crossChromosome:

private String crossChromosome(String m, String f) {
    if (rnd.nextInt(BOUND) >= crossRate * BOUND) {
        return (rnd.nextInt(2) == 1) ? f : m;
    }

    char[] mArr = m.toCharArray();
    char[] fArr = f.toCharArray();
    final int mLength = mArr.length;
    final int fLength = fArr.length;
    final int crossPoint = rnd.nextInt(mLength - 2) + 2;
    final int nextCrossPoint = rnd.nextInt(mLength - crossPoint) + crossPoint;

    StringBuilder chrString = new StringBuilder();

    for (int i = 0; i < fLength; i++) {
        boolean condition = (i < crossPoint);

        if (crType != 1 && !condition) {
            condition = (i >= nextCrossPoint);
        }
        char ch = condition ? fArr[i] : mArr[i];
        chrString.append(ch);
    }

    return chrString.toString();
}

I put condition inside the loop and used the ternary operator.
Same suggestions for the method mutateChromosome, below my version:

private String mutateChromosome(String k) {
    final int mutatRateMulBound = mutatRate * BOUND;
    char[] kArr = k.toCharArray();

    StringBuilder chrString = new StringBuilder();

    for (char c : kArr) {
        boolean condition = rnd.nextInt(BOUND) < mutatRateMulBound;
        char ch = condition ? options.charAt(rnd.nextInt(27)) : c;
        chrString.append(ch);
    }

    return chrString.toString();
}


Enumerating an alphabet Since train rides can be long and boring, I've decided to make use of that time and fiddle around with some good ol' C.

What I did was to create a way to enumerate words of a certain length of an arbitrary alphabet. Basically, suppose you have an alphabet

{ '0', '1' }

(ooh, binary!) and want to get all words of length 7, you'd call

enumerate("01", 7);

and have a nice array of pointers containing the strings "0000000" to "1111111". What for, you ask? Dunno. Stuff.

I've written it on my phone, so it may be a tad slimmer than the usual 80 characters. It's also not split into files for the very same reason.

#include <math.h>
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char *translate(
  unsigned long value,
  const char *alphabet,
  size_t length
) {
  size_t base;
  char *result;

  if (!alphabet || (base = strlen(alphabet)) < 2) {
    return NULL;
  }

  if (length == 0) {
    length = 1 + floor(log(value) / log(base));
  }

  result = malloc(length + 1);

  if (!result) {
    return NULL;
  }

  memset(result, alphabet[0], length);
  result[length] = 0;

  for (int i = length - 1; i >= 0 && value > 0; --i) {
    unsigned long mod = value % base;
    result[i] = alphabet[mod];

    value -= mod;
    value /= base;
  }

  return result;
}

char **enumerate(
  const char *alphabet,
  const size_t length
) {
  size_t base;

  if (!alphabet || (base = strlen(alphabet)) < 2) {
    return NULL;
  }

  if (length == 0) {
    return NULL;
  }

  unsigned long end = pow(base, length);
  char **array = malloc((end + 1) * sizeof(char*));

  if (!array) {
    return NULL;
  }

  array[end] = NULL;

  for (int i = 0; i != end; ++i) {
    char *storage = malloc(length + 1);
    char *translated = translate(i, alphabet, length);

    if (!storage || !translated) {
      if (storage) {
        free(storage);
      }

      if (translated) {
        free(translated);
      }

      array[i] = NULL;
      goto error_cleanup;
    }

    strcpy(storage, translated);
    free(translated);
    array[i] = storage;
  }

  return array;

  error_cleanup:

  for (int i = 0; array[i] != NULL; ++i) {
    free(array[i]);
  }

  free(array);
  return NULL;
}

Example usage; simply concatenate the listings.

int main(int argc, char *argv[]) {
  if (argc < 2) {
    printf("Usage: %s <length> <alphabet>",
          argv[0]);
    exit(0);
  }

  size_t length = atoi(argv[1]);
  char *alphabet = argv[2];

  char **array = enumerate(alphabet, length);

  if (!array) {
    printf("Error while enumerating.");
    exit(1);
  }

  for (int i = 0; array[i] != NULL; ++i) {
    printf("%s\n", array[i]);

    free(array[i]);
  }

  free(array);
  exit(0);
}

What I'm most concerned about is idiomaticality (is that even a word?) of the code, since my C is rusty and self-taught, especially concerning the use of that string array. Also, is my goto used well and with good reason?

If you want to improve usability, you should introduce some kind of type for your array (typedef the pointer, or introduce some type of struct that will contain useful metadata) and provide the appropriate free function for it. This way, the user doesn't have to free all the elements themselves and can just call my_free(&my_type_ptr) when he's done. Your error_cleanup should just call this function as well.
This bit of code does unnecessary work:

char *storage = malloc(length + 1);
char *translated = translate(i, alphabet, length);

if (!storage || !translated) {
  if (storage) {
    free(storage);
  }

  if (translated) {
    free(translated);
  }

  array[i] = NULL;
  goto error_cleanup;
}

strcpy(storage, translated);
free(translated);
array[i] = storage;

in the form of allocating an extra block of memory and copying things there from an already allocated block of memory. Simplify to:

array[i] = translate(i, alphabet, length);
if (array[i] == NULL) {
    // Error in translating, abort
    goto error_cleanup;
}

  is my goto used well and with good reason?

No. You only have one user of the goto. If you look above at the more concise version of your function, it's completely reasonable just to say "no more goto" and replace it with free_word_array(array); return NULL;. Any time you have a need to clean multiple resources on any failure, goto can be a good choice.
(argc < 2) needs to be (argc < 3). As it is written, running your program with only one extra argument ./main 6 results in Error while enumerating. instead of printing the help message. Remember, the program invocation is the 0th argument but counts in the count as well.
I prefer return from main rather than exit.
translate could use more comments within the algorithm, for quicker reading/understanding. It looks like you're just enumerating all possible words within a given set, and this recovers the word.
Maybe it makes sense not to have an array of arrays, but rather an array of numbers that you then convert to the string representation if the user wants to print it? But then the logical conclusion of that is that you don't really need to allocate memory at all then: you can just as easily return the number of permutations, and work out the appropriate requested permutation when it is requested.
Your code will only work with single-wide characters, and produce gibberish for any alphabet that contains characters that aren't (think UTF-8, for instance, where the character can be wider than a byte, meaning that you'll shuffle the character incorrectly). This might not be in your requirements, but thought I'd mention it since this always plagues non-ASCII character stuff in C.