Skip to content

Refactor: Extract duplicated code patterns into reusable helpers #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Copilot wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Refactor: Extract duplicated code patterns into reusable helpers #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7e87e79
Initial plan
Copilot Dec 9, 2025
a80be6f
Refactor: Extract duplicate token wrapping code in sexp.rs
Copilot Dec 9, 2025
b95614a
Refactor: Extract duplicate sequential statement evaluation in eval.rs
Copilot Dec 9, 2025
0af41d5
Refactor: Extract duplicate binary operations in eval.rs
Copilot Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 57 additions & 59 deletions src/eval.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,49 @@ fn eval_sexp_to_num(sexp: Rc<Sexp>, state: &mut EvalState) -> i64 {
}
}

// Helper function to evaluate a sequence of statements and return the last result
fn eval_statements_sequentially(statements: &[Rc<Sexp>], state: &mut EvalState) -> Rc<Sexp> {
let len = statements.len();

if len == 0 {
return Rc::new(Sexp::Nil);
}

let mut result = Rc::new(Sexp::Nil);
for (i, statement) in statements.iter().enumerate() {
result = eval_sexp(statement.clone(), state);
if i < len - 1 {
// Only continue evaluating if not the last statement
continue;
}
}
result
}

// Helper function to perform binary arithmetic operations
fn eval_binary_arithmetic_op<F>(l: &[Rc<Sexp>], state: &mut EvalState, op: F) -> Rc<Sexp>
where
F: Fn(i64, i64) -> i64,
{
let left = eval_sexp_to_num(l[1].clone(), state);
let right = eval_sexp_to_num(l[2].clone(), state);
Rc::new(Sexp::Num(op(left, right)))
}

// Helper function to perform binary comparison operations
fn eval_binary_comparison_op<F>(l: &[Rc<Sexp>], state: &mut EvalState, op: F) -> Rc<Sexp>
where
F: Fn(i64, i64) -> bool,
{
let left = eval_sexp_to_num(l[1].clone(), state);
let right = eval_sexp_to_num(l[2].clone(), state);
if op(left, right) {
Rc::new(Sexp::True)
} else {
Rc::new(Sexp::Nil)
}
}

fn get_from_fn_map_or_global(var_name: &str, state: &EvalState) -> Option<Rc<Sexp>> {
// Check function scopes first for variable shadowing
for var_map in state.fn_map.iter().rev() {
Expand Down Expand Up @@ -137,23 +180,9 @@ pub(crate) fn eval_sexp(sexp: Rc<Sexp>, state: &mut EvalState) -> Rc<Sexp> {
// Skip the actual "let", and the subsequent paramlist,
// then eval all remaining and return last
let statements = &l[2..];
let len = statements.len();

if len > 0 {
let mut result = Rc::new(Sexp::Nil);
for (i, statement) in statements.iter().enumerate() {
result = eval_sexp(statement.clone(), state);
if i < len - 1 {
// Only continue evaluating if not the last statement
continue;
}
}
state.fn_map.pop();
return result;
} else {
state.fn_map.pop();
Rc::new(Sexp::Nil)
}
let result = eval_statements_sequentially(statements, state);
state.fn_map.pop();
result
}

Sexp::Instrinsics(IntrinsicInstruction::Cdr) => {
Expand Down Expand Up @@ -216,21 +245,12 @@ pub(crate) fn eval_sexp(sexp: Rc<Sexp>, state: &mut EvalState) -> Rc<Sexp> {
Sexp::Instrinsics(IntrinsicInstruction::Progn) => {
// Skip the actual "progn", then eval all and return last
let statements = &l[1..];
let len = statements.len();

if len == 0 {

if statements.is_empty() {
panic!("progn should have at least 2 arguments");
}

let mut result = Rc::new(Sexp::Nil);
for (i, statement) in statements.iter().enumerate() {
result = eval_sexp(statement.clone(), state);
if i < len - 1 {
// Only continue evaluating if not the last statement
continue;
}
}
result

eval_statements_sequentially(statements, state)
}
Sexp::Instrinsics(IntrinsicInstruction::Defmacro) => {
let macro_name = match *l[1] {
Expand Down Expand Up @@ -361,35 +381,13 @@ pub(crate) fn eval_sexp(sexp: Rc<Sexp>, state: &mut EvalState) -> Rc<Sexp> {
}
result
}
"-" => Rc::new(Sexp::Num(
eval_sexp_to_num(l[1].clone(), state) - eval_sexp_to_num(l[2].clone(), state),
)),
"+" => Rc::new(Sexp::Num(
eval_sexp_to_num(l[1].clone(), state) + eval_sexp_to_num(l[2].clone(), state),
)),
"*" => Rc::new(Sexp::Num(
eval_sexp_to_num(l[1].clone(), state) * eval_sexp_to_num(l[2].clone(), state),
)),
"<" => {
if eval_sexp_to_num(l[1].clone(), state) < eval_sexp_to_num(l[2].clone(), state) {
Rc::new(Sexp::True)
} else {
Rc::new(Sexp::Nil)
}
}
">" => {
if eval_sexp_to_num(l[1].clone(), state) > eval_sexp_to_num(l[2].clone(), state) {
Rc::new(Sexp::True)
} else {
Rc::new(Sexp::Nil)
}
}
"%" => Rc::new(Sexp::Num(
eval_sexp_to_num(l[1].clone(), state) % eval_sexp_to_num(l[2].clone(), state),
)),
"/" => Rc::new(Sexp::Num(
eval_sexp_to_num(l[1].clone(), state) / eval_sexp_to_num(l[2].clone(), state),
)),
"-" => eval_binary_arithmetic_op(l, state, |a, b| a - b),
"+" => eval_binary_arithmetic_op(l, state, |a, b| a + b),
"*" => eval_binary_arithmetic_op(l, state, |a, b| a * b),
"<" => eval_binary_comparison_op(l, state, |a, b| a < b),
">" => eval_binary_comparison_op(l, state, |a, b| a > b),
"%" => eval_binary_arithmetic_op(l, state, |a, b| a % b),
"/" => eval_binary_arithmetic_op(l, state, |a, b| a / b),
"setq" => {
if let Sexp::Sym(ref s) = *l[1] {
let eval_value = eval_sexp(l[2].clone(), state);
Expand Down
66 changes: 28 additions & 38 deletions src/sexp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,32 @@ impl std::fmt::Display for Sexp {
}
}

// Helper function to wrap tokens with special characters (quote/backtick/comma)
fn wrap_tokens_with_special_chars(
expanded_tokens: &mut Vec<lex::Token>,
apostrophe_or_backtick_vec: &[lex::Token],
) {
for i in apostrophe_or_backtick_vec.iter() {
match i {
lex::Token::Apostrophe => {
expanded_tokens.push(lex::Token::LeftParen);
expanded_tokens.push(lex::Token::String("quote".to_string()));
}
lex::Token::Backtick => {
expanded_tokens.push(lex::Token::LeftParen);
expanded_tokens.push(lex::Token::String("backtick".to_string()));
}
lex::Token::Comma => {
expanded_tokens.push(lex::Token::LeftParen);
expanded_tokens.push(lex::Token::String("comma".to_string()));
}
_ => {
panic!("invalid syntax")
}
}
}
}

// This expands backticks into the quotation syntax
fn expand_special_characters(tokens: &[lex::Token]) -> Vec<lex::Token> {
let mut expanded_tokens: Vec<lex::Token> = vec![];
Expand Down Expand Up @@ -179,25 +205,7 @@ fn expand_special_characters(tokens: &[lex::Token]) -> Vec<lex::Token> {

let slice = &tokens[index + 1..index + skip + 2].to_vec().clone();

for i in apostrophe_or_backtick_vec.iter() {
match i {
lex::Token::Apostrophe => {
expanded_tokens.push(lex::Token::LeftParen);
expanded_tokens.push(lex::Token::String("quote".to_string()));
}
lex::Token::Backtick => {
expanded_tokens.push(lex::Token::LeftParen);
expanded_tokens.push(lex::Token::String("backtick".to_string()));
}
lex::Token::Comma => {
expanded_tokens.push(lex::Token::LeftParen);
expanded_tokens.push(lex::Token::String("comma".to_string()));
}
_ => {
panic!("invalid syntax")
}
}
}
wrap_tokens_with_special_chars(&mut expanded_tokens, &apostrophe_or_backtick_vec);

let mut expanded = expand_special_characters(slice);

Expand All @@ -218,25 +226,7 @@ fn expand_special_characters(tokens: &[lex::Token]) -> Vec<lex::Token> {
}
// If it's a symbol or a digit, then we need to wrap it in a quote
lex::Token::String(_) | lex::Token::Digit(_) => {
for i in apostrophe_or_backtick_vec.iter() {
match i {
lex::Token::Apostrophe => {
expanded_tokens.push(lex::Token::LeftParen);
expanded_tokens.push(lex::Token::String("quote".to_string()));
}
lex::Token::Backtick => {
expanded_tokens.push(lex::Token::LeftParen);
expanded_tokens.push(lex::Token::String("backtick".to_string()));
}
lex::Token::Comma => {
expanded_tokens.push(lex::Token::LeftParen);
expanded_tokens.push(lex::Token::String("comma".to_string()));
}
_ => {
panic!("invalid syntax")
}
}
}
wrap_tokens_with_special_chars(&mut expanded_tokens, &apostrophe_or_backtick_vec);

expanded_tokens.push(next_token.clone());

Expand Down